Merge pull request #1205 from alphagov/handle-lists-as-placeholder-values

Handle lists as CSV field values

app/__init__.py

@@ -30,7 +30,7 @@ from notifications_python_client.errors import HTTPError
 from notifications_utils import logging, request_id, formatters
 from notifications_utils.clients.statsd.statsd_client import StatsdClient
 from notifications_utils.recipients import validate_phone_number, InvalidPhoneError
-from notifications_utils.field import escape_html
+from notifications_utils.formatters import formatted_list
 from pygments import highlight
 from pygments.formatters.html import HtmlFormatter
 from pygments.lexers.javascript import JavascriptLexer
@@ -359,33 +359,6 @@ def format_notification_status_as_url(status):
     }.get(status)
 
 
-def formatted_list(
-    items,
-    conjunction='and',
-    before_each='‘',
-    after_each='’',
-    separator=', ',
-    prefix='',
-    prefix_plural=''
-):
-    if prefix:
-        prefix += ' '
-    if prefix_plural:
-        prefix_plural += ' '
-
-    items = list(map(escape_html, items))
-    if len(items) == 1:
-        return Markup('{prefix}{before_each}{items[0]}{after_each}'.format(**locals()))
-    elif items:
-        formatted_items = ['{}{}{}'.format(before_each, item, after_each) for item in items]
-
-        first_items = separator.join(formatted_items[:-1])
-        last_item = formatted_items[-1]
-        return Markup((
-            '{prefix_plural}{first_items} {conjunction} {last_item}'
-        ).format(**locals()))
-
-
 def nl2br(value):
     return formatters.nl2br(value) if value else ''
 

app/main/views/templates.py

@@ -15,7 +15,7 @@ from flask_login import login_required, current_user
 from flask_weasyprint import HTML, render_pdf
 from dateutil.parser import parse
 
-from notifications_utils.field import escape_html
+from notifications_utils.formatters import escape_html
 from notifications_utils.template import LetterPreviewTemplate
 from notifications_utils.recipients import first_column_headings
 from notifications_python_client.errors import HTTPError

app/templates/components/table.html

@@ -81,7 +81,15 @@
 
 {% macro text_field(text, status='') -%}
   {% call field(status=status) %}
-    {{ text }}
+    {% if text is iterable and text is not string %}
+      <ul class="list list-bullet">
+        {% for item in text %}
+          <li>{{ item }}</li>
+        {% endfor %}
+      </ul>
+    {% else %}
+      {{ text }}
+    {% endif %}
   {% endcall %}
 {%- endmacro %}
 

app/templates/views/check.html

@@ -205,20 +205,14 @@
           </span>
         {% endcall %}
       {% elif item['columns'][column].ignore %}
-        {% call field(status='default') %}
-          {{ item['columns'][column].data if item['columns'][column].data != None }}
-        {% endcall %}
+        {{ text_field(item['columns'][column].data or '', status='default') }}
       {% else %}
-        {% call field() %}
-          {{ item['columns'][column].data if item['columns'][column].data != None }}
-        {% endcall %}
+        {{ text_field(item['columns'][column].data or '') }}
       {% endif %}
     {% endfor %}
     {% if item['columns'].get(None) %}
       {% for column in item['columns'][None].data %}
-        {% call field(status='default') %}
-          {{ column }}
-        {% endcall %}
+        {{ text_field(column, status='default') }}
       {% endfor %}
     {% endif %}
   {% endcall %}

requirements.txt

@@ -31,4 +31,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@13.10.0#egg=notifications-utils==13.10.0
+git+https://github.com/alphagov/notifications-utils.git@14.0.0#egg=notifications-utils==14.0.0

tests/app/main/views/test_send.py

@@ -14,6 +14,7 @@ from flask import url_for
 from app.main.views.send import get_check_messages_back_url
 
 from tests import validate_route_permission, template_json
+from tests.app.test_utils import normalize_spaces
 
 template_types = ['email', 'sms']
 
@@ -233,6 +234,47 @@ def test_upload_valid_csv_shows_page_title(
     assert page.h1.text.strip() == 'Preview of Two week reminder'
 
 
+def test_upload_valid_csv_shows_file_contents(
+    logged_in_client,
+    mocker,
+    mock_get_service_template_with_placeholders,
+    mock_s3_upload,
+    mock_get_users_by_service,
+    mock_get_detailed_service_for_today,
+    service_one,
+    fake_uuid,
+):
+
+    mocker.patch('app.main.views.send.s3download', return_value="""
+        phone number,name,thing,thing,thing
+        07700900986, Jo,  foo,  foo,  foo
+    """)
+
+    response = logged_in_client.post(
+        url_for('main.send_messages', service_id=service_one['id'], template_id=fake_uuid),
+        data={'file': (BytesIO(''.encode('utf-8')), 'valid.csv')},
+        follow_redirects=True,
+    )
+
+    assert response.status_code == 200
+    page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
+    for index, cell in enumerate([
+        '<td class="table-field-index"> <span>2</span> </td>',
+        '<td class="table-field-center-aligned "> <div class=""> 07700900986 </div> </td>',
+        '<td class="table-field-center-aligned "> <div class=""> Jo </div> </td>',
+        (
+            '<td class="table-field-center-aligned "> '
+            '<div class="table-field-status-default"> '
+            '<ul class="list list-bullet"> '
+            '<li>foo</li> <li>foo</li> <li>foo</li> '
+            '</ul> '
+            '</div> '
+            '</td>'
+        ),
+    ]):
+        assert normalize_spaces(str(page.select('table tbody td')[index])) == cell
+
+
 def test_send_test_sms_message(
     logged_in_client,
     mocker,

tests/app/test_jinja_filters.py

@@ -1,23 +0,0 @@
-import pytest
-
-from flask import Markup
-from app import formatted_list
-
-
-@pytest.mark.parametrize('items, kwargs, expected_output', [
-    ([1], {}, '‘1’'),
-    ([1, 2], {}, '‘1’ and ‘2’'),
-    ([1, 2, 3], {}, '‘1’, ‘2’ and ‘3’'),
-    ([1, 2, 3], {'prefix': 'foo', 'prefix_plural': 'bar'}, 'bar ‘1’, ‘2’ and ‘3’'),
-    ([1], {'prefix': 'foo', 'prefix_plural': 'bar'}, 'foo ‘1’'),
-    ([1, 2, 3], {'before_each': 'a', 'after_each': 'b'}, 'a1b, a2b and a3b'),
-    ([1, 2, 3], {'conjunction': 'foo'}, '‘1’, ‘2’ foo ‘3’'),
-    (['&'], {'before_each': '<i>', 'after_each': '</i>'}, '<i>&amp;</i>'),
-    ([1, 2, 3], {'before_each': '<i>', 'after_each': '</i>'}, '<i>1</i>, <i>2</i> and <i>3</i>'),
-])
-def test_formatted_list(items, kwargs, expected_output):
-    assert formatted_list(items, **kwargs) == expected_output
-
-
-def test_formatted_list_returns_markup():
-    assert isinstance(formatted_list([0]), Markup)