From dc3f26a646f258a7d94be9bdebd856d773919749 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Fri, 7 Jul 2017 15:34:06 +0100 Subject: [PATCH 1/3] Make conversation page update using AJAX Fairly self-explanatory. Uses the same pattern of breaking things up into functions as the jobs page. --- app/main/views/conversation.py | 25 ++++++++++- .../views/conversations/conversation.html | 43 +++++-------------- .../views/conversations/messages.html | 34 +++++++++++++++ tests/app/main/views/test_conversation.py | 26 ++++++++++- 4 files changed, 93 insertions(+), 35 deletions(-) create mode 100644 app/templates/views/conversations/messages.html diff --git a/app/main/views/conversation.py b/app/main/views/conversation.py index 84b6528be..9d21e873a 100644 --- a/app/main/views/conversation.py +++ b/app/main/views/conversation.py @@ -1,4 +1,5 @@ from flask import ( + jsonify, render_template, url_for, ) @@ -20,11 +21,33 @@ def conversation(service_id, notification_id): return render_template( 'views/conversations/conversation.html', - conversation=get_sms_thread(service_id, user_number=user_number), user_number=user_number, + partials=get_conversation_partials(service_id, user_number), + updates_url=url_for('.conversation_updates', service_id=service_id, notification_id=notification_id), ) +@main.route("/services//conversation/.json") +@login_required +@user_has_permissions('view_activity', admin_override=True) +def conversation_updates(service_id, notification_id): + + return jsonify(get_conversation_partials( + service_id, + get_user_number(service_id, notification_id) + )) + + +def get_conversation_partials(service_id, user_number): + + return { + 'messages': render_template( + 'views/conversations/messages.html', + conversation=get_sms_thread(service_id, user_number), + ) + } + + def get_user_number(service_id, notification_id): try: user_number = service_api_client.get_inbound_sms_by_id(service_id, notification_id)['user_number'] diff --git a/app/templates/views/conversations/conversation.html b/app/templates/views/conversations/conversation.html index cd181bd37..6028ea791 100644 --- a/app/templates/views/conversations/conversation.html +++ b/app/templates/views/conversations/conversation.html @@ -1,7 +1,9 @@ +{% from "components/ajax-block.html" import ajax_block %} + {% extends "withnav_template.html" %} {% block service_page_title %} - Conversation + {{ user_number }} {% endblock %} {% block maincolumn_content %} @@ -13,38 +15,13 @@ {{ user_number }} - {% for message in conversation %} -
- {% if message.inbound %} -
- {{ message.content | string }} -
- {{ message.created_at | format_datetime_relative }} -
-
- {% else %} -
-   -
-
- {{ message.content | string }} - {% if message.status == 'delivered' %} -
- {{ message.created_at | format_datetime_relative }} -
- {% elif message.status in ['pending', 'sending', 'created'] %} -
- sending -
- {% else %} -
- Failed (sent {{ message.created_at | format_datetime_relative }}) -
- {% endif %} -
- {% endif %} -
- {% endfor %} + + {{ ajax_block( + partials, + updates_url, + 'messages', + ) }} + {% endblock %} diff --git a/app/templates/views/conversations/messages.html b/app/templates/views/conversations/messages.html new file mode 100644 index 000000000..d4b754485 --- /dev/null +++ b/app/templates/views/conversations/messages.html @@ -0,0 +1,34 @@ +
+ {% for message in conversation %} +
+ {% if message.inbound %} +
+ {{ message.content | string }} +
+ {{ message.created_at | format_datetime_relative }} +
+
+ {% else %} +
+   +
+
+ {{ message.content | string }} + {% if message.status == 'delivered' %} +
+ {{ message.created_at | format_datetime_relative }} +
+ {% elif message.status in ['pending', 'sending', 'created'] %} +
+ sending +
+ {% else %} +
+ Failed (sent {{ message.created_at | format_datetime_relative }}) +
+ {% endif %} +
+ {% endif %} +
+ {% endfor %} +
diff --git a/tests/app/main/views/test_conversation.py b/tests/app/main/views/test_conversation.py index 605054c32..7d8333d39 100644 --- a/tests/app/main/views/test_conversation.py +++ b/tests/app/main/views/test_conversation.py @@ -1,3 +1,4 @@ +import json import pytest from bs4 import BeautifulSoup @@ -95,7 +96,6 @@ def test_view_conversation( 'main.conversation', service_id=SERVICE_ONE_ID, notification_id=fake_uuid, - _test_page_title=False, ) messages = page.select('.sms-message-wrapper') @@ -162,3 +162,27 @@ def test_view_conversation( normalize_spaces(messages[index].text), normalize_spaces(statuses[index].text), ) == expected + + +def test_view_conversation_updates( + logged_in_client, + mocker, + fake_uuid, + mock_get_notification, +): + + mock_get_partials = mocker.patch( + 'app.main.views.conversation.get_conversation_partials', + return_value={'messages': 'foo'} + ) + + response = logged_in_client.get(url_for( + 'main.conversation_updates', + service_id=SERVICE_ONE_ID, + notification_id=fake_uuid, + )) + + assert response.status_code == 200 + assert json.loads(response.get_data(as_text=True)) == {'messages': 'foo'} + + mock_get_partials.assert_called_once_with(SERVICE_ONE_ID, '07123 456789') From 5f6351762a5e6e27520b6b343553250f29f83cf3 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Fri, 7 Jul 2017 15:51:59 +0100 Subject: [PATCH 2/3] Make inbox page update using AJAX Fairly self-explanatory. Uses the same pattern of breaking things up into functions as the jobs page. --- app/main/views/dashboard.py | 23 ++++++++-- .../views/dashboard/_inbox_messages.html | 37 ++++++++++++++++ app/templates/views/dashboard/inbox.html | 44 ++++--------------- tests/app/main/views/test_dashboard.py | 25 +++++++++++ 4 files changed, 90 insertions(+), 39 deletions(-) create mode 100644 app/templates/views/dashboard/_inbox_messages.html diff --git a/app/main/views/dashboard.py b/app/main/views/dashboard.py index 2832b330c..992902115 100644 --- a/app/main/views/dashboard.py +++ b/app/main/views/dashboard.py @@ -143,6 +143,23 @@ def monthly(service_id): @user_has_permissions('view_activity', admin_override=True) 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), + ) + + +@main.route("/services//inbox.json") +@login_required +@user_has_permissions('view_activity', admin_override=True) +def inbox_updates(service_id): + + return jsonify(get_inbox_partials(service_id)) + + +def get_inbox_partials(service_id): + if 'inbound_sms' not in current_service['permissions']: abort(403) @@ -156,12 +173,12 @@ def inbox(service_id): }: messages_to_show.append(message) - return render_template( - 'views/dashboard/inbox.html', + return {'messages': render_template( + 'views/dashboard/_inbox_messages.html', messages=messages_to_show, count_of_messages=len(inbound_messages), count_of_users=len(messages_to_show), - ) + )} def aggregate_usage(template_statistics, sort_key='count'): diff --git a/app/templates/views/dashboard/_inbox_messages.html b/app/templates/views/dashboard/_inbox_messages.html new file mode 100644 index 000000000..95af9ffab --- /dev/null +++ b/app/templates/views/dashboard/_inbox_messages.html @@ -0,0 +1,37 @@ +{% from "components/table.html" import list_table, field, hidden_field_heading, right_aligned_field_heading, row_heading %} +{% from "components/message-count-label.html" import message_count_label %} + +
+ {% call(item, row_number) list_table( + messages, + caption="Inbox", + caption_visible=False, + empty_message='When users text your service’s phone number ({}) you’ll see the messages here'.format(current_service.sms_sender), + field_headings=[ + 'From', + 'First two lines of message' + ], + field_headings_visible=False + ) %} + {% call field() %} + + {{ item.user_number | format_phone_number_human_readable }} + + {{ item.content }} + {% endcall %} + {% call field(align='right') %} + + {{ item.created_at | format_delta }} + + {% endcall %} + {% endcall %} + {% if 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/app/templates/views/dashboard/inbox.html b/app/templates/views/dashboard/inbox.html index 6dbc2fb5b..39189c3de 100644 --- a/app/templates/views/dashboard/inbox.html +++ b/app/templates/views/dashboard/inbox.html @@ -1,5 +1,4 @@ -{% from "components/table.html" import list_table, field, hidden_field_heading, right_aligned_field_heading, row_heading %} -{% from "components/message-count-label.html" import message_count_label %} +{% from "components/ajax-block.html" import ajax_block %} {% extends "withnav_template.html" %} @@ -12,38 +11,11 @@

Received text messages

-
- {% call(item, row_number) list_table( - messages, - caption="Inbox", - caption_visible=False, - empty_message='When users text your service’s phone number ({}) you’ll see the messages here'.format(current_service.sms_sender), - field_headings=[ - 'From', - 'First two lines of message' - ], - field_headings_visible=False - ) %} - {% call field() %} - - {{ item.user_number | format_phone_number_human_readable }} - - {{ item.content }} - {% endcall %} - {% call field(align='right') %} - - {{ item.created_at | format_delta }} - - {% endcall %} - {% endcall %} - {% if 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 %} -
+ + {{ ajax_block( + partials, + updates_url, + 'messages', + ) }} + {% endblock %} diff --git a/tests/app/main/views/test_dashboard.py b/tests/app/main/views/test_dashboard.py index 9e0c60985..a4560fbfa 100644 --- a/tests/app/main/views/test_dashboard.py +++ b/tests/app/main/views/test_dashboard.py @@ -1,3 +1,4 @@ +import json from functools import partial import copy from unittest.mock import call, ANY @@ -220,6 +221,30 @@ def test_anyone_can_see_inbox( ) +def test_view_inbox_updates( + logged_in_client, + service_one, + mocker, + mock_get_inbound_sms_with_no_messages, +): + + service_one['permissions'] = ['inbound_sms'] + + mock_get_partials = mocker.patch( + 'app.main.views.dashboard.get_inbox_partials', + return_value={'messages': 'foo'}, + ) + + response = logged_in_client.get(url_for( + 'main.inbox_updates', service_id=SERVICE_ONE_ID, + )) + + assert response.status_code == 200 + assert json.loads(response.get_data(as_text=True)) == {'messages': 'foo'} + + mock_get_partials.assert_called_once_with(SERVICE_ONE_ID) + + def test_should_show_recent_templates_on_dashboard( logged_in_client, mocker, From cfbe767dff29d518daca39f1133f108dec6e7a5d Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Wed, 12 Jul 2017 14:39:50 +0100 Subject: [PATCH 3/3] Test permissions on the inbox updates --- tests/app/main/views/test_dashboard.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/tests/app/main/views/test_dashboard.py b/tests/app/main/views/test_dashboard.py index a4560fbfa..4a0074bd2 100644 --- a/tests/app/main/views/test_dashboard.py +++ b/tests/app/main/views/test_dashboard.py @@ -189,12 +189,17 @@ def test_empty_inbox( ) +@pytest.mark.parametrize('endpoint', [ + 'main.inbox', + 'main.inbox_updates', +]) def test_inbox_not_accessible_to_service_without_permissions( logged_in_client, service_one, + endpoint, ): service_one['permissions'] = [] - response = logged_in_client.get(url_for('main.inbox', service_id=SERVICE_ONE_ID)) + response = logged_in_client.get(url_for(endpoint, service_id=SERVICE_ONE_ID)) assert response.status_code == 403 @@ -228,8 +233,6 @@ def test_view_inbox_updates( mock_get_inbound_sms_with_no_messages, ): - service_one['permissions'] = ['inbound_sms'] - mock_get_partials = mocker.patch( 'app.main.views.dashboard.get_inbox_partials', return_value={'messages': 'foo'},