From 61fd27c4f61e318dd23f6f9a49f7bd1a306e68f6 Mon Sep 17 00:00:00 2001
From: Chris Hill-Scott <me@quis.cc>
Date: Fri, 7 Jul 2017 10:04:49 +0100
Subject: [PATCH] Fix 500 on two-way conversation page
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

We changed the schema used by the endpoint that searches for
notifications by recipient. So the admin app was looking for the wrong
thing in the JSON.

This is hard to catch in tests because it relies on our fixtures
matching what the API really returns.

This commit fixes the code to use the correct key to lookup the template
content from the JSON.

This also exposed the fact that we weren’t passing in the
personalisation any more (perhaps got lost in the re-reverts somehow)
so users were only seeing the template in the inbound view, not the
full message content.
---
 app/main/views/conversation.py            | 10 ++++++--
 tests/__init__.py                         |  1 -
 tests/app/main/views/test_conversation.py | 30 +++++++++++++++++------
 3 files changed, 31 insertions(+), 10 deletions(-)

diff --git a/app/main/views/conversation.py b/app/main/views/conversation.py
index 5542e25de..84b6528be 100644
--- a/app/main/views/conversation.py
+++ b/app/main/views/conversation.py
@@ -41,14 +41,20 @@ def get_sms_thread(service_id, user_number):
     ), key=lambda notification: notification['created_at']):
 
         is_inbound = ('notify_number' in notification)
+        redact_personalisation = notification.get('template', {}).get('redact_personalisation', False)
+
+        if redact_personalisation:
+            notification['personalisation'] = {}
 
         yield {
             'inbound': is_inbound,
             'content': SMSPreviewTemplate(
                 {
-                    'content': notification.get('content') or notification['body']
+                    'content': notification.get('content') or notification['template']['content']
                 },
-                downgrade_non_gsm_characters=(not is_inbound)
+                notification.get('personalisation'),
+                downgrade_non_gsm_characters=(not is_inbound),
+                redact_missing_personalisation=redact_personalisation,
             ),
             'created_at': notification['created_at'],
             'status': notification.get('status'),
diff --git a/tests/__init__.py b/tests/__init__.py
index 33a46087d..ab20273e6 100644
--- a/tests/__init__.py
+++ b/tests/__init__.py
@@ -254,7 +254,6 @@ def notification_json(
         'notifications': [{
             'id': uuid.uuid4(),
             'to': to,
-            'body': template['content'],
             'template': template,
             'job': job_payload,
             'sent_at': sent_at,
diff --git a/tests/app/main/views/test_conversation.py b/tests/app/main/views/test_conversation.py
index 0408690d5..014e7810d 100644
--- a/tests/app/main/views/test_conversation.py
+++ b/tests/app/main/views/test_conversation.py
@@ -7,8 +7,9 @@ from flask import (
 from notifications_python_client.errors import HTTPError
 from tests.conftest import (
     SERVICE_ONE_ID,
+    mock_get_notifications,
+    normalize_spaces,
 )
-from tests.conftest import normalize_spaces
 from freezegun import freeze_time
 from unittest import mock
 from app.main.views.conversation import get_user_number
@@ -70,15 +71,30 @@ def test_get_user_phone_number_raises_if_both_API_requests_fail(
     mock_get_notification.assert_called_once_with('service', 'notification')
 
 
+@pytest.mark.parametrize('outbound_redacted, expected_outbound_content', [
+    (True, 'Hello hidden'),
+    (False, 'Hello Jo'),
+])
 @freeze_time("2012-01-01 00:00:00")
 def test_view_conversation(
     logged_in_client,
+    mocker,
+    api_user_active,
     fake_uuid,
     mock_get_notification,
     mock_get_inbound_sms,
-    mock_get_notifications,
+    outbound_redacted,
+    expected_outbound_content,
 ):
 
+    mock_get_notifications(
+        mocker,
+        api_user_active,
+        template_content='Hello ((name))',
+        personalisation={'name': 'Jo'},
+        redact_personalisation=outbound_redacted,
+    )
+
     response = logged_in_client.get(url_for(
         'main.conversation',
         service_id=SERVICE_ONE_ID,
@@ -127,23 +143,23 @@ def test_view_conversation(
             'Failed (sent yesterday at 11:00pm)',
         ),
         (
-            'template content',
+            expected_outbound_content,
             'yesterday at midnight',
         ),
         (
-            'template content',
+            expected_outbound_content,
             'yesterday at midnight',
         ),
         (
-            'template content',
+            expected_outbound_content,
             'yesterday at midnight',
         ),
         (
-            'template content',
+            expected_outbound_content,
             'yesterday at midnight',
         ),
         (
-            'template content',
+            expected_outbound_content,
             'yesterday at midnight',
         ),
     ]):