Merge pull request #1291 from alphagov/two-way

Add page to show two-way conversation

app/__init__.py

@@ -257,13 +257,20 @@ def format_datetime_relative(date):
 
 
 def get_human_day(time):
+
     #  Add 1 hour to get ‘midnight today’ instead of ‘midnight tomorrow’
-    time = (gmt_timezones(time) - timedelta(hours=1)).strftime('%A')
+    time_as_day = (gmt_timezones(time) - timedelta(hours=1)).strftime('%A')
-    if time == datetime.utcnow().strftime('%A'):
+    six_days_ago = gmt_timezones((datetime.utcnow() + timedelta(days=-6)).isoformat())
-        return 'today'
+
-    if time == (datetime.utcnow() + timedelta(days=1)).strftime('%A'):
+    if gmt_timezones(time) < six_days_ago:
+        return format_date_short(time)
+    if time_as_day == (datetime.utcnow() + timedelta(days=1)).strftime('%A'):
         return 'tomorrow'
-    return time
+    if time_as_day == datetime.utcnow().strftime('%A'):
+        return 'today'
+    if time_as_day == (datetime.utcnow() + timedelta(days=-1)).strftime('%A'):
+        return 'yesterday'
+    return format_date_short(time)
 
 
 def format_time(date):

app/assets/stylesheets/components/sms-message.scss

@@ -19,7 +19,7 @@ $tail-angle: 20deg;
     content: "";
     display: block;
     position: absolute;
-    bottom: -4px;
+    bottom: -5px;
     right: -20px;
     border: 10px solid transparent;
     border-left-width: 13px;
@@ -31,8 +31,45 @@ $tail-angle: 20deg;
 
 }
 
+.sms-message-inbound {
+
+  .sms-message-wrapper {
+
+    &:after {
+      border-left-color: transparent;
+      border-bottom-color: $panel-colour;
+      border-right-color: $panel-colour;
+      right: auto;
+      left: -20px;
+      transform: rotate(-$tail-angle);
+    }
+
+  }
+
+}
+
 .sms-message-recipient {
   @include copy-19;
   color: $secondary-text-colour;
   margin: 10px 0 0 0;
 }
+
+.sms-message-status {
+  @include core-16;
+  color: $secondary-text-colour;
+  margin: -20px $gutter-half 20px $gutter-half;
+}
+
+.sms-message-status-outbound {
+  text-align: right;
+}
+
+.sms-message-row {
+
+  &:focus {
+    outline: none;
+    padding-top: 120px;
+    margin-top: -120px;
+  }
+
+}

app/main/__init__.py

@@ -27,5 +27,6 @@ from app.main.views import (
     feedback,
     providers,
     platform_admin,
-    letter_jobs
+    letter_jobs,
+    conversation,
 )

app/main/views/conversation.py

@@ -0,0 +1,56 @@
+from flask import (
+    render_template,
+    url_for,
+)
+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.utils import user_has_permissions
+from app import notification_api_client, service_api_client
+from notifications_python_client.errors import HTTPError
+
+
+@main.route("/services/<service_id>/conversation/<notification_id>")
+@login_required
+@user_has_permissions('view_activity', admin_override=True)
+def conversation(service_id, notification_id):
+
+    user_number = get_user_number(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,
+    )
+
+
+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']
+    except HTTPError:
+        user_number = notification_api_client.get_notification(service_id, notification_id)['to']
+    return format_phone_number_human_readable(user_number)
+
+
+def get_sms_thread(service_id, user_number):
+
+    for notification in sorted((
+        notification_api_client.get_notifications_for_service(service_id, to=user_number)['notifications'] +
+        service_api_client.get_inbound_sms(service_id, user_number=user_number)
+    ), key=lambda notification: notification['created_at']):
+
+        is_inbound = ('notify_number' in notification)
+
+        yield {
+            'inbound': is_inbound,
+            'content': SMSPreviewTemplate(
+                {
+                    'content': notification.get('content') or notification['template']['content']
+                },
+                downgrade_non_gsm_characters=(not is_inbound)
+            ),
+            'created_at': notification['created_at'],
+            'status': notification.get('status'),
+            'id': notification['id'],
+        }

app/notify_client/notification_api_client.py

@@ -54,3 +54,8 @@ class NotificationApiClient(NotifyAdminAPIClient):
                 url='/service/{}/notifications'.format(service_id),
                 params=params
             )
+
+    def get_notification(self, service_id, notification_id):
+        return self.get(
+            url='/service/{}/notifications/{}'.format(service_id, notification_id)
+        )

app/notify_client/service_api_client.py

@@ -3,6 +3,7 @@ from __future__ import unicode_literals
 from flask import url_for
 from app.utils import BrowsableItem
 from app.notify_client import _attach_current_user, NotifyAdminAPIClient
+from . import notification_api_client
 
 
 class ServiceAPIClient(NotifyAdminAPIClient):
@@ -244,11 +245,22 @@ class ServiceAPIClient(NotifyAdminAPIClient):
             params=dict(year=year)
         )
 
-    def get_inbound_sms(self, service_id):
+    def get_inbound_sms(self, service_id, user_number=''):
         return self.get(
-            '/service/{}/inbound-sms'.format(service_id)
+            '/service/{}/inbound-sms?user_number={}'.format(
+                service_id,
+                user_number,
+            )
         )['data']
 
+    def get_inbound_sms_by_id(self, service_id, notification_id):
+        return self.get(
+            '/service/{}/inbound-sms/{}'.format(
+                service_id,
+                notification_id,
+            )
+        )
+
     def get_inbound_sms_summary(self, service_id):
         return self.get(
             '/service/{}/inbound-sms/summary'.format(service_id)

app/templates/views/conversations/conversation.html

@@ -0,0 +1,50 @@
+{% extends "withnav_template.html" %}
+
+{% block service_page_title %}
+  Conversation
+{% endblock %}
+
+{% block maincolumn_content %}
+
+  <div class="dashboard">
+
+    <div class="bottom-gutter js-stick-at-top-when-scrolling">
+      <h1 class="heading-large">
+        {{ user_number }}
+      </h1>
+    </div>
+    {% for message in conversation %}
+      <div class="grid-row sms-message-row" id="n{{ message.id }}" tabindex="0">
+        {% if message.inbound %}
+          <div class="column-two-thirds sms-message-inbound">
+            {{ message.content | string }}
+            <div class="sms-message-status">
+              {{ message.created_at | format_datetime_relative }}
+            </div>
+          </div>
+        {% else %}
+          <div class="column-one-third">
+            &nbsp;
+          </div>
+          <div class="column-two-thirds">
+            {{ message.content | string }}
+            {% if message.status == 'delivered' %}
+              <div class="sms-message-status sms-message-status-outbound">
+                {{ message.created_at | format_datetime_relative }}
+              </div>
+            {% elif message.status in ['pending', 'sending', 'created'] %}
+              <div class="sms-message-status sms-message-status-outbound hint">
+                sending
+              </div>
+            {% else %}
+              <div class="sms-message-status sms-message-status-outbound table-field-error-label">
+                Failed (sent {{ message.created_at | format_datetime_relative }})
+              </div>
+            {% endif %}
+          </div>
+        {% endif %}
+      </div>
+    {% endfor %}
+  </div>
+
+{% endblock %}

app/templates/views/dashboard/inbox.html

@@ -25,7 +25,12 @@
       field_headings_visible=False
     ) %}
       {% call field() %}
-        <span class="file-list-filename" href="#">{{ item.user_number | format_phone_number_human_readable }}</span>
+        <a
+          class="file-list-filename"
+          href="{{ url_for('.conversation', service_id=current_service.id, notification_id=item.id) }}#n{{ item.id }}"
+        >
+          {{ item.user_number | format_phone_number_human_readable }}
+        </a>
         <span class="wide-left-hand-column">{{ item.content }}</span>
       {% endcall %}
       {% call field(align='right') %}

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.2.0#egg=notifications-utils==17.2.0
+git+https://github.com/alphagov/notifications-utils.git@17.3.0#egg=notifications-utils==17.3.0

tests/__init__.py

@@ -248,7 +248,9 @@ def notification_json(
             'template': {
                 'id': template['id'],
                 'name': template['name'],
-                'template_type': template['template_type']},
+                'template_type': template['template_type'],
+                'content': template['content'],
+            },
             'job': job_payload,
             'sent_at': sent_at,
             'status': status,

tests/app/main/views/test_conversation.py

@@ -0,0 +1,153 @@
+import pytest
+from bs4 import BeautifulSoup
+
+from flask import (
+    url_for,
+)
+from notifications_python_client.errors import HTTPError
+from tests.conftest import (
+    SERVICE_ONE_ID,
+)
+from tests.app.test_utils import normalize_spaces
+from freezegun import freeze_time
+from unittest import mock
+from app.main.views.conversation import get_user_number
+
+
+@mock.patch(
+    'app.main.views.conversation.service_api_client.get_inbound_sms_by_id',
+    return_value={
+        'user_number': '4407900900123'
+    }
+)
+@mock.patch(
+    'app.main.views.conversation.notification_api_client.get_notification',
+    side_effect=HTTPError,
+)
+def test_get_user_phone_number_when_only_inbound_exists(
+    mock_get_notification,
+    mock_get_inbound_sms,
+):
+    assert get_user_number('service', 'notification') == '07900 900123'
+    mock_get_inbound_sms.assert_called_once_with('service', 'notification')
+    assert mock_get_notification.called is False
+
+
+@mock.patch(
+    'app.main.views.conversation.service_api_client.get_inbound_sms_by_id',
+    side_effect=HTTPError,
+)
+@mock.patch(
+    'app.main.views.conversation.notification_api_client.get_notification',
+    return_value={
+        'to': '15550000000'
+    }
+)
+def test_get_user_phone_number_when_only_outbound_exists(
+    mock_get_notification,
+    mock_get_inbound_sms,
+):
+    assert get_user_number('service', 'notification') == '+1 555-000-0000'
+    mock_get_inbound_sms.assert_called_once_with('service', 'notification')
+    mock_get_notification.assert_called_once_with('service', 'notification')
+
+
+@mock.patch(
+    'app.main.views.conversation.service_api_client.get_inbound_sms_by_id',
+    side_effect=HTTPError,
+)
+@mock.patch(
+    'app.main.views.conversation.notification_api_client.get_notification',
+    side_effect=HTTPError,
+)
+def test_get_user_phone_number_raises_if_both_API_requests_fail(
+    mock_get_notification,
+    mock_get_inbound_sms,
+):
+    with pytest.raises(HTTPError):
+        get_user_number('service', 'notification')
+    mock_get_inbound_sms.assert_called_once_with('service', 'notification')
+    mock_get_notification.assert_called_once_with('service', 'notification')
+
+
+@freeze_time("2012-01-01 00:00:00")
+def test_view_conversation(
+    logged_in_client,
+    fake_uuid,
+    mock_get_notification,
+    mock_get_inbound_sms,
+    mock_get_notifications,
+):
+
+    response = logged_in_client.get(url_for(
+        'main.conversation',
+        service_id=SERVICE_ONE_ID,
+        notification_id=fake_uuid,
+    ))
+    assert response.status_code == 200
+    page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
+
+    messages = page.select('.sms-message-wrapper')
+    statuses = page.select('.sms-message-status')
+
+    assert len(messages) == 13
+    assert len(statuses) == 13
+
+    for index, expected in enumerate([
+        (
+            'message-8',
+            'Failed (sent yesterday at 2:59pm)',
+        ),
+        (
+            'message-7',
+            'Failed (sent yesterday at 2:59pm)',
+        ),
+        (
+            'message-6',
+            'Failed (sent yesterday at 4:59pm)',
+        ),
+        (
+            'message-5',
+            'Failed (sent yesterday at 6:59pm)',
+        ),
+        (
+            'message-4',
+            'Failed (sent yesterday at 8:59pm)',
+        ),
+        (
+            'message-3',
+            'Failed (sent yesterday at 10:59pm)',
+        ),
+        (
+            'message-2',
+            'Failed (sent yesterday at 10:59pm)',
+        ),
+        (
+            'message-1',
+            'Failed (sent yesterday at 11:00pm)',
+        ),
+        (
+            'template content',
+            'yesterday at midnight',
+        ),
+        (
+            'template content',
+            'yesterday at midnight',
+        ),
+        (
+            'template content',
+            'yesterday at midnight',
+        ),
+        (
+            'template content',
+            'yesterday at midnight',
+        ),
+        (
+            'template content',
+            'yesterday at midnight',
+        ),
+    ]):
+        assert (
+            normalize_spaces(messages[index].text),
+            normalize_spaces(statuses[index].text),
+        ) == expected

tests/conftest.py

@@ -1096,6 +1096,22 @@ def mock_get_notifications(mocker, api_user_active):
     )
 
 
+@pytest.fixture(scope='function')
+def mock_get_notification(mocker, api_user_active):
+    def _get_notification(
+        service_id,
+        notification_id,
+    ):
+        return single_notification_json(
+            service_id,
+        )
+
+    return mocker.patch(
+        'app.notification_api_client.get_notification',
+        side_effect=_get_notification
+    )
+
+
 @pytest.fixture(scope='function')
 def mock_get_notifications_with_previous_next(mocker):
     def _get_notifications(service_id,
@@ -1138,11 +1154,13 @@ def mock_get_notifications_with_no_notifications(mocker):
 def mock_get_inbound_sms(mocker):
     def _get_inbound_sms(
         service_id,
+        user_number=None,
     ):
         return [{
             'user_number': '0790090000' + str(i),
             'content': 'message-{}'.format(index + 1),
-            'created_at': (datetime.utcnow() - timedelta(minutes=60 * (i + 1))).isoformat()
+            '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(