From 88f169d970fd5ca39975dc98cbee6ab53638cf1d Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Sun, 15 Oct 2017 12:59:36 +0100 Subject: [PATCH 1/4] Add a page to pick a template to reply with MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If your service is receiving inbound messages, you might want to reply to them. Without an API integration, the only way to do this is by doing the send one off flow. But currently there is no direct link from the page which shows the inbound messages. So this commit: - adds a link to a new page… - …which shows the available (ie only text message) templates with which you can reply --- app/main/views/conversation.py | 25 +++++++++ .../views/conversations/conversation.html | 6 ++ .../views/templates/choose-reply.html | 56 +++++++++++++++++++ tests/app/main/views/test_conversation.py | 48 ++++++++++++++++ 4 files changed, 135 insertions(+) create mode 100644 app/templates/views/templates/choose-reply.html diff --git a/app/main/views/conversation.py b/app/main/views/conversation.py index 8f32fba1e..ffda0f6e4 100644 --- a/app/main/views/conversation.py +++ b/app/main/views/conversation.py @@ -7,6 +7,7 @@ from flask_login import login_required from notifications_utils.recipients import format_phone_number_human_readable from notifications_utils.template import SMSPreviewTemplate from app.main import main +from app.main.forms import SearchTemplatesForm from app.utils import user_has_permissions from app import notification_api_client, service_api_client from notifications_python_client.errors import HTTPError @@ -24,6 +25,7 @@ def conversation(service_id, notification_id): 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), + notification_id=notification_id, ) @@ -38,6 +40,29 @@ def conversation_updates(service_id, notification_id): )) +@main.route("/services//conversation//reply-with") +@login_required +@user_has_permissions('send_texts', admin_override=True) +def conversation_reply( + service_id, + notification_id, +): + + templates = [ + template + for template in service_api_client.get_service_templates(service_id)['data'] + if template['template_type'] == 'sms' + ] + + return render_template( + 'views/templates/choose-reply.html', + templates=templates, + show_search_box=(len(templates) > 7), + template_type='sms', + search_form=SearchTemplatesForm(), + ) + + def get_conversation_partials(service_id, user_number): return { diff --git a/app/templates/views/conversations/conversation.html b/app/templates/views/conversations/conversation.html index 6028ea791..b46b57950 100644 --- a/app/templates/views/conversations/conversation.html +++ b/app/templates/views/conversations/conversation.html @@ -22,6 +22,12 @@ 'messages', ) }} + {% if current_user.has_permissions(['send_texts'], admin_override=True) %} +

+ Send a text message to this phone number +

+ {% endif %} + {% endblock %} diff --git a/app/templates/views/templates/choose-reply.html b/app/templates/views/templates/choose-reply.html new file mode 100644 index 000000000..3094748b2 --- /dev/null +++ b/app/templates/views/templates/choose-reply.html @@ -0,0 +1,56 @@ +{% from "components/pill.html" import pill %} +{% from "components/message-count-label.html" import message_count_label %} +{% from "components/textbox.html" import textbox %} + +{% extends "withnav_template.html" %} + +{% block service_page_title %} + Choose a template +{% endblock %} + +{% block maincolumn_content %} + +

Choose a template

+ + {% if not templates %} + + {% if current_user.has_permissions(permissions=['manage_templates'], any_=True) %} +

+ You need a template before you can send text messages. +

+ Add a new template + {% else %} +

+ You need to ask your service manager to add templates before you + can send text messages. +

+ {% endif %} + + {% else %} + + {% if show_search_box %} +
+
+ {{ textbox( + search_form.search, + width='1-1' + ) }} +
+
+ {% endif %} + + + {% endif %} + +{% endblock %} diff --git a/tests/app/main/views/test_conversation.py b/tests/app/main/views/test_conversation.py index 5076b9bb5..b5b296a02 100644 --- a/tests/app/main/views/test_conversation.py +++ b/tests/app/main/views/test_conversation.py @@ -214,3 +214,51 @@ def test_view_conversation_with_empty_inbound( messages = page.select('.sms-message-wrapper') assert len(messages) == 1 + + +def test_conversation_links_to_reply( + client_request, + fake_uuid, + mock_get_notification, + mock_get_notifications, + mock_get_inbound_sms, +): + page = client_request.get( + 'main.conversation', + service_id=SERVICE_ONE_ID, + notification_id=fake_uuid, + ) + + assert page.select('main p')[-1].select_one('a')['href'] == ( + url_for( + '.conversation_reply', + service_id=SERVICE_ONE_ID, + notification_id=fake_uuid, + ) + ) + + +def test_conversation_reply_shows_templates( + client_request, + fake_uuid, + mock_get_service_templates, +): + page = client_request.get( + 'main.conversation_reply', + service_id=SERVICE_ONE_ID, + notification_id=fake_uuid, + ) + + for index, expected in enumerate([ + 'sms_template_one', + 'sms_template_two', + ]): + link = page.select('.message-name')[index] + assert normalize_spaces(link.text) == expected + assert link.select_one('a')['href'].startswith( + url_for( + 'main.view_template', + service_id=SERVICE_ONE_ID, + template_id='', + ) + ) From 91cdc9ffcba6d847ea7eee0eae21da8e130fd364 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Sun, 15 Oct 2017 13:40:49 +0100 Subject: [PATCH 2/4] Auto populate the phone number when replying MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently the only way of ‘replying’ to an inbound message is by copy/pasting the phone number from the inbound page into the send one off page. Rather than making people copy/paste, this commit puts the phone number in the session, if the user is going through the new reply flow. --- app/main/views/conversation.py | 23 ++++++++++++++++ .../views/templates/choose-reply.html | 2 +- tests/app/main/views/test_conversation.py | 26 ++++++++++++++++++- 3 files changed, 49 insertions(+), 2 deletions(-) diff --git a/app/main/views/conversation.py b/app/main/views/conversation.py index ffda0f6e4..abdd416de 100644 --- a/app/main/views/conversation.py +++ b/app/main/views/conversation.py @@ -1,5 +1,7 @@ from flask import ( jsonify, + session, + redirect, render_template, url_for, ) @@ -60,9 +62,30 @@ def conversation_reply( show_search_box=(len(templates) > 7), template_type='sms', search_form=SearchTemplatesForm(), + notification_id=notification_id, ) +@main.route("/services//conversation//reply-with/") +@login_required +@user_has_permissions('send_texts', admin_override=True) +def conversation_reply_with_template( + service_id, + notification_id, + template_id, +): + + session['recipient'] = get_user_number(service_id, notification_id) + session['placeholders'] = {'phone number': session['recipient']} + + return redirect(url_for( + 'main.send_one_off_step', + service_id=service_id, + template_id=template_id, + step_index=1, + )) + + def get_conversation_partials(service_id, user_number): return { diff --git a/app/templates/views/templates/choose-reply.html b/app/templates/views/templates/choose-reply.html index 3094748b2..11927b0c6 100644 --- a/app/templates/views/templates/choose-reply.html +++ b/app/templates/views/templates/choose-reply.html @@ -43,7 +43,7 @@ {% for template in templates %}

- {{ template.name }} + {{ template.name }}

{{ message_count_label(1, template.template_type, suffix='')|capitalize }} template diff --git a/tests/app/main/views/test_conversation.py b/tests/app/main/views/test_conversation.py index b5b296a02..1da8caf3b 100644 --- a/tests/app/main/views/test_conversation.py +++ b/tests/app/main/views/test_conversation.py @@ -257,8 +257,32 @@ def test_conversation_reply_shows_templates( assert normalize_spaces(link.text) == expected assert link.select_one('a')['href'].startswith( url_for( - 'main.view_template', + 'main.conversation_reply_with_template', service_id=SERVICE_ONE_ID, + notification_id=fake_uuid, template_id='', ) ) + + +def test_conversation_reply_redirects_with_phone_number_from_notification( + client_request, + fake_uuid, + mock_get_notification, + mock_get_service_template, +): + + page = client_request.get( + 'main.conversation_reply_with_template', + service_id=SERVICE_ONE_ID, + notification_id=fake_uuid, + template_id=fake_uuid, + _follow_redirects=True, + ) + + for element, expected_text in [ + ('h1', 'Preview of Two week reminder'), + ('.sms-message-recipient', 'To: 07123 456789'), + ('.sms-message-wrapper', 'service one: Template content with & entity'), + ]: + assert normalize_spaces(page.select_one(element).text) == expected_text From 05513c6fa7203aea490e104ec1a69cb799c6f081 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Sun, 15 Oct 2017 14:17:23 +0100 Subject: [PATCH 3/4] Refactor to reuse search box HTML --- app/templates/views/templates/_search-box.html | 10 ++++++++++ app/templates/views/templates/choose-reply.html | 11 +---------- app/templates/views/templates/choose.html | 11 +---------- 3 files changed, 12 insertions(+), 20 deletions(-) create mode 100644 app/templates/views/templates/_search-box.html diff --git a/app/templates/views/templates/_search-box.html b/app/templates/views/templates/_search-box.html new file mode 100644 index 000000000..d150ebebe --- /dev/null +++ b/app/templates/views/templates/_search-box.html @@ -0,0 +1,10 @@ + {% if show_search_box %} +

+
+ {{ textbox( + search_form.search, + width='1-1' + ) }} +
+
+ {% endif %} \ No newline at end of file diff --git a/app/templates/views/templates/choose-reply.html b/app/templates/views/templates/choose-reply.html index 11927b0c6..a356d0100 100644 --- a/app/templates/views/templates/choose-reply.html +++ b/app/templates/views/templates/choose-reply.html @@ -28,16 +28,7 @@ {% else %} - {% if show_search_box %} -
-
- {{ textbox( - search_form.search, - width='1-1' - ) }} -
-
- {% endif %} + {% include "views/templates/_search-box.html" %}
{% endif %} - {% if show_search_box %} -
-
- {{ textbox( - search_form.search, - width='1-1' - ) }} -
-
- {% endif %} + {% include "views/templates/_search-box.html" %}