Merge branch 'master' into styleguide-l

app/main/views/jobs.py

@@ -1,6 +1,4 @@
 # -*- coding: utf-8 -*-
-from orderedset import OrderedSet
-from itertools import chain
 
 from flask import (
     render_template,
@@ -18,7 +16,6 @@ from notifications_utils.template import (
     Template,
     WithSubjectTemplate,
 )
-from werkzeug.datastructures import MultiDict
 
 from app import (
     job_api_client,
@@ -35,39 +32,12 @@ from app.utils import (
     user_has_permissions,
     generate_notifications_csv,
     get_time_left,
-    REQUESTED_STATUSES,
-    FAILURE_STATUSES,
-    SENDING_STATUSES,
-    DELIVERED_STATUSES,
     get_letter_timings,
+    parse_filter_args, set_status_filters
 )
 from app.statistics_utils import add_rate_to_job
 
 
-def _parse_filter_args(filter_dict):
-    if not isinstance(filter_dict, MultiDict):
-        filter_dict = MultiDict(filter_dict)
-
-    return MultiDict(
-        (
-            key,
-            (','.join(filter_dict.getlist(key))).split(',')
-        )
-        for key in filter_dict.keys()
-        if ''.join(filter_dict.getlist(key))
-    )
-
-
-def _set_status_filters(filter_args):
-    status_filters = filter_args.get('status', [])
-    return list(OrderedSet(chain(
-        (status_filters or REQUESTED_STATUSES),
-        DELIVERED_STATUSES if 'delivered' in status_filters else [],
-        SENDING_STATUSES if 'sending' in status_filters else [],
-        FAILURE_STATUSES if 'failed' in status_filters else []
-    )))
-
-
 @main.route("/services/<service_id>/jobs")
 @login_required
 @user_has_permissions('view_activity', admin_override=True)
@@ -104,8 +74,8 @@ def view_job(service_id, job_id):
     if job['job_status'] == 'cancelled':
         abort(404)
 
-    filter_args = _parse_filter_args(request.args)
+    filter_args = parse_filter_args(request.args)
-    filter_args['status'] = _set_status_filters(filter_args)
+    filter_args['status'] = set_status_filters(filter_args)
 
     total_notifications = job.get('notification_count', 0)
     processed_notifications = job.get('notifications_delivered', 0) + job.get('notifications_failed', 0)
@@ -146,8 +116,8 @@ def view_job_csv(service_id, job_id):
         template_id=job['template'],
         version=job['template_version']
     )['data']
-    filter_args = _parse_filter_args(request.args)
+    filter_args = parse_filter_args(request.args)
-    filter_args['status'] = _set_status_filters(filter_args)
+    filter_args['status'] = set_status_filters(filter_args)
 
     return Response(
         stream_with_context(
@@ -206,6 +176,12 @@ def view_notifications(service_id, message_type):
         page=request.args.get('page', 1),
         to=request.form.get('to', ''),
         search_form=SearchNotificationsForm(to=request.form.get('to', '')),
+        download_link=url_for(
+            '.download_notifications_csv',
+            service_id=current_service['id'],
+            message_type=message_type,
+            status=request.args.get('status')
+        )
     )
 
 
@@ -226,8 +202,8 @@ def get_notifications(service_id, message_type, status_override=None):
         abort(404, "Invalid page argument ({}) reverting to page 1.".format(request.args['page'], None))
     if message_type not in ['email', 'sms', 'letter']:
         abort(404)
-    filter_args = _parse_filter_args(request.args)
+    filter_args = parse_filter_args(request.args)
-    filter_args['status'] = _set_status_filters(filter_args)
+    filter_args['status'] = set_status_filters(filter_args)
     if request.path.endswith('csv'):
         return Response(
             generate_notifications_csv(
@@ -250,7 +226,6 @@ def get_notifications(service_id, message_type, status_override=None):
         limit_days=current_app.config['ACTIVITY_STATS_LIMIT_DAYS'],
         to=request.form.get('to', ''),
     )
-
     url_args = {
         'message_type': message_type,
         'status': request.args.get('status')
@@ -361,8 +336,8 @@ def _get_job_counts(job):
 
 
 def get_job_partials(job, template):
-    filter_args = _parse_filter_args(request.args)
+    filter_args = parse_filter_args(request.args)
-    filter_args['status'] = _set_status_filters(filter_args)
+    filter_args['status'] = set_status_filters(filter_args)
     notifications = notification_api_client.get_notifications_for_service(
         job['service'], job['id'], status=filter_args['status']
     )

app/main/views/notifications.py

@@ -1,18 +1,20 @@
 # -*- coding: utf-8 -*-
+from datetime import datetime
+
 from flask import (
     abort,
     render_template,
     jsonify,
     request,
     url_for,
-)
+    Response, stream_with_context)
 from flask_login import login_required
 
 from app import (
     notification_api_client,
     job_api_client,
-    current_service
+    current_service,
-)
+    format_date_numeric)
 from app.main import main
 from app.template_previews import TemplatePreview, get_page_count_for_letter
 from app.utils import (
@@ -23,7 +25,7 @@ from app.utils import (
     get_letter_timings,
     FAILURE_STATUSES,
     DELIVERED_STATUSES,
-)
+    generate_notifications_csv, parse_filter_args, set_status_filters)
 
 
 @main.route("/services/<service_id>/notification/<uuid:notification_id>")
@@ -138,3 +140,31 @@ def get_all_personalisation_from_notification(notification):
         notification['personalisation']['phone_number'] = notification['to']
 
     return notification['personalisation']
+
+
+@main.route("/services/<service_id>/download-notifications.csv")
+@login_required
+@user_has_permissions('view_activity', admin_override=True)
+def download_notifications_csv(service_id):
+    filter_args = parse_filter_args(request.args)
+    filter_args['status'] = set_status_filters(filter_args)
+
+    return Response(
+        stream_with_context(
+            generate_notifications_csv(
+                service_id=service_id,
+                job_id=None,
+                status=filter_args.get('status'),
+                page=request.args.get('page', 1),
+                page_size=5000,
+                format_for_csv=True
+            )
+        ),
+        mimetype='text/csv',
+        headers={
+            'Content-Disposition': 'inline; filename="{} - {} - {} report.csv"'.format(
+                format_date_numeric(datetime.now().strftime("%Y-%m-%dT%H:%M:%S.%fZ")),
+                filter_args['message_type'][0],
+                current_service['name'])
+        }
+    )

app/templates/views/api/callbacks.html

@@ -30,4 +30,8 @@
 	    {% endcall %}
 	  {% endcall %}
 	</div>
+  {{ page_footer(
+    secondary_link=url_for('.api_integration', service_id=current_service.id),
+    secondary_link_text='Back to API integration'
+  ) }}
 {% endblock %}

app/templates/views/notifications.html

@@ -42,8 +42,16 @@
       <input type="hidden" name="to" value="{{ search_form.to.data }}">
       <input type="hidden" name="csrf_token" value="{{ csrf_token() }}">
     </form>
+  {% else %}
+    <form id="search-form" method="post">
+      <input type="hidden" name="csrf_token" value="{{ csrf_token() }}"/>
+    </form>
   {% endif %}
-
+    <!--<p class="bottom-gutter">-->
+       <!--<a href="{{ download_link }}" download="download" class="heading-small">Download this report</a>-->
+       <!--&emsp;-->
+       <!--Data available for 7 days-->
+    <!--</p>-->
   {{ ajax_block(
     partials,
     url_for('.get_notifications_as_json', service_id=current_service.id, message_type=message_type, status=status, page=page),

app/templates/views/service-settings/name.html

@@ -10,13 +10,16 @@
 
   <h1 class="heading-large">Change your service name</h1>
 
-
   <div class="form-group">
-    <p>Users will see your service name:</p>
+    {% if current_service.prefix_sms %}
-    <ul class="list-bullet">
+      <p>Users will see your service name:</p>
-      <li>at the start of every text message, eg ‘Vehicle tax: we received your payment, thank you’</li>
+      <ul class="list-bullet">
-      <li>as your email sender name</li>
+        <li>at the start of every text message, eg ‘{{ current_service.name }}: This is an example message’</li>
-    </ul>
+        <li>as your email sender name</li>
+      </ul>
+    {% else %}
+      <p>Users will see your service name as your email sender name.</p>
+    {% endif %}
   </div>
 
   <form method="post">

app/templates/views/service-settings/set-inbound-sms.html

@@ -19,6 +19,12 @@
           If you want to turn this feature off,
           <a href="{{ url_for('.support') }}">get in touch with the GOV.UK Notify team</a>.
         </p>
+        {% if current_user.has_permissions(['manage_api_keys'], admin_override=True) %}
+          <p>
+            You can set up callbacks for received text messages on the
+            <a href="{{ url_for('.api_callbacks', service_id=current_service.id) }}">API integration page</a>.
+          </p>
+        {% endif %}
       {% else %}
         <p>
           Receiving text messages from your users is an

app/utils.py

@@ -1,5 +1,7 @@
 import re
 import csv
+from itertools import chain
+
 import pytz
 from io import StringIO
 from os import path
@@ -28,7 +30,8 @@ from notifications_utils.template import (
     LetterImageTemplate,
     LetterPreviewTemplate,
 )
-
+from orderedset._orderedset import OrderedSet
+from werkzeug.datastructures import MultiDict
 
 SENDING_STATUSES = ['created', 'pending', 'sending']
 DELIVERED_STATUSES = ['delivered', 'sent']
@@ -125,28 +128,47 @@ def get_errors_for_csv(recipients, template_type):
 
 def generate_notifications_csv(**kwargs):
     from app import notification_api_client
-
     if 'page' not in kwargs:
         kwargs['page'] = 1
-    fieldnames = ['Row number', 'Recipient', 'Template', 'Type', 'Job', 'Status', 'Time']
+
+    if kwargs['job_id']:
+        fieldnames = ['Row number', 'Recipient', 'Template', 'Type', 'Job', 'Status', 'Time']
+    else:
+        fieldnames = ['Recipient', 'Template', 'Type', 'Job', 'Status', 'Time']
+
     yield ','.join(fieldnames) + '\n'
 
     while kwargs['page']:
+        # if job_id then response looks different
         notifications_resp = notification_api_client.get_notifications_for_service(**kwargs)
         notifications = notifications_resp['notifications']
-        for notification in notifications:
+        if kwargs['job_id']:
-            values = [
+            for notification in notifications:
-                notification['row_number'],
+                    values = [
-                notification['recipient'],
+                        notification['row_number'],
-                notification['template_name'],
+                        notification['recipient'],
-                notification['template_type'],
+                        notification['template_name'],
-                notification['job_name'],
+                        notification['template_type'],
-                notification['status'],
+                        notification['job_name'],
-                notification['created_at']
+                        notification['status'],
-            ]
+                        notification['created_at']
-            line = ','.join(str(i) for i in values) + '\n'
+                    ]
-            yield line
+                    line = ','.join(str(i) for i in values) + '\n'
-
+                    yield line
+        else:
+            # Change here
+            for notification in notifications:
+                values = [
+                    notification['to'],
+                    notification['template']['name'],
+                    notification['template']['template_type'],
+                    notification.get('job_name', None),
+                    notification['status'],
+                    notification['created_at'],
+                    notification['updated_at']
+                ]
+                line = ','.join(str(i) for i in values) + '\n'
+                yield line
         if notifications_resp['links'].get('next'):
             kwargs['page'] += 1
         else:
@@ -382,3 +404,27 @@ def get_cdn_domain():
     domain = parsed_uri.netloc[len(subdomain + '.'):]
 
     return "static-logos.{}".format(domain)
+
+
+def parse_filter_args(filter_dict):
+    if not isinstance(filter_dict, MultiDict):
+        filter_dict = MultiDict(filter_dict)
+
+    return MultiDict(
+        (
+            key,
+            (','.join(filter_dict.getlist(key))).split(',')
+        )
+        for key in filter_dict.keys()
+        if ''.join(filter_dict.getlist(key))
+    )
+
+
+def set_status_filters(filter_args):
+    status_filters = filter_args.get('status', [])
+    return list(OrderedSet(chain(
+        (status_filters or REQUESTED_STATUSES),
+        DELIVERED_STATUSES if 'delivered' in status_filters else [],
+        SENDING_STATUSES if 'sending' in status_filters else [],
+        FAILURE_STATUSES if 'failed' in status_filters else []
+    )))

requirements.txt

@@ -5,8 +5,8 @@ Flask-Login==0.4.1
 
 boto3==1.5.12
 blinker==1.4
-pyexcel==0.5.6
+pyexcel==0.5.7
-pyexcel-io==0.5.5
+pyexcel-io==0.5.6
 pyexcel-xls==0.5.5
 pyexcel-xlsx==0.5.5
 pyexcel-ods3==0.5.2

requirements_for_test.txt

@@ -2,7 +2,7 @@
 pytest==3.3.2
 pytest-mock==1.6.3
 pytest-cov==2.5.1
-pytest-xdist==1.21.0
+pytest-xdist==1.22.0
 coveralls==1.2.0
 httpretty==0.8.14
 beautifulsoup4==4.6.0

tests/app/main/views/test_service_settings.py

@@ -11,6 +11,7 @@ from app.utils import email_safe
 from tests import validate_route_permission, service_json
 from tests.conftest import (
     active_user_with_permissions,
+    active_user_no_api_key_permission,
     platform_admin_user,
     normalize_spaces,
     multiple_reply_to_email_addresses,
@@ -236,16 +237,27 @@ def test_escapes_letter_contact_block(
 
 
 def test_should_show_service_name(
-        logged_in_client,
+    client_request,
-        service_one,
 ):
-    response = logged_in_client.get(url_for(
+    page = client_request.get('main.service_name_change', service_id=SERVICE_ONE_ID)
-        'main.service_name_change', service_id=service_one['id']))
-    assert response.status_code == 200
-    page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
     assert page.find('h1').text == 'Change your service name'
     assert page.find('input', attrs={"type": "text"})['value'] == 'service one'
-    app.service_api_client.get_service.assert_called_with(service_one['id'])
+    assert page.select_one('main p').text == 'Users will see your service name:'
+    assert normalize_spaces(page.select_one('main ul').text) == (
+        'at the start of every text message, eg ‘service one: This is an example message’ '
+        'as your email sender name'
+    )
+    app.service_api_client.get_service.assert_called_with(SERVICE_ONE_ID)
+
+
+def test_should_show_service_name_with_no_prefixing(
+    client_request,
+    service_one,
+):
+    service_one['prefix_sms'] = False
+    page = client_request.get('main.service_name_change', service_id=SERVICE_ONE_ID)
+    assert page.find('h1').text == 'Change your service name'
+    assert page.select_one('main p').text == 'Users will see your service name as your email sender name.'
 
 
 def test_should_redirect_after_change_service_name(
@@ -1916,6 +1928,41 @@ def test_set_inbound_sms_when_inbound_number_is_not_set(
     assert response.status_code == 200
 
 
+@pytest.mark.parametrize('user, expected_paragraphs', [
+    (active_user_with_permissions, [
+        'Your service can receive text messages sent to 07700900123.',
+        'If you want to turn this feature off, get in touch with the GOV.UK Notify team.',
+        'You can set up callbacks for received text messages on the API integration page.',
+    ]),
+    (active_user_no_api_key_permission, [
+        'Your service can receive text messages sent to 07700900123.',
+        'If you want to turn this feature off, get in touch with the GOV.UK Notify team.',
+    ]),
+])
+def test_set_inbound_sms_when_inbound_number_is_set(
+    client,
+    service_one,
+    mocker,
+    fake_uuid,
+    user,
+    expected_paragraphs,
+):
+    service_one['permissions'] = ['inbound_sms']
+    mocker.patch('app.inbound_number_client.get_inbound_sms_number_for_service', return_value={
+        'data': {'number': '07700900123'}
+    })
+    client.login(user(fake_uuid), mocker, service_one)
+    response = client.get(url_for(
+        'main.service_set_inbound_sms', service_id=SERVICE_ONE_ID
+    ))
+    paragraphs = BeautifulSoup(response.data.decode('utf-8'), 'html.parser').select('main p')
+
+    assert len(paragraphs) == len(expected_paragraphs)
+
+    for index, p in enumerate(expected_paragraphs):
+        assert normalize_spaces(paragraphs[index].text) == p
+
+
 def test_empty_letter_contact_block_returns_error(
     logged_in_client,
     service_one,

tests/app/test_utils.py

@@ -15,6 +15,7 @@ from app.utils import (
     get_letter_timings,
     get_cdn_domain
 )
+from tests.conftest import fake_uuid
 
 
 def _get_notifications_csv(
@@ -28,7 +29,8 @@ def _get_notifications_csv(
     status='Delivered',
     created_at='Thursday 19 April at 12:00',
     rows=1,
-    with_links=False
+    with_links=False,
+    job_id=fake_uuid
 ):
     links = {}
     if with_links:
@@ -41,12 +43,15 @@ def _get_notifications_csv(
     data = {
         'notifications': [{
             "row_number": row_number,
+            "to": recipient,
             "recipient": recipient,
             "template_name": template_name,
             "template_type": template_type,
+            "template": {"name": template_name, "template_type": template_type},
             "job_name": job_name,
             "status": status,
-            "created_at": created_at
+            "created_at": created_at,
+            "updated_at": None
         } for i in range(rows)],
         'total': rows,
         'page_size': 50,
@@ -58,7 +63,8 @@ def _get_notifications_csv(
 @pytest.fixture(scope='function')
 def _get_notifications_csv_mock(
     mocker,
-    api_user_active
+    api_user_active,
+    job_id=fake_uuid
 ):
     return mocker.patch(
         'app.notification_api_client.get_notifications_for_service',
@@ -122,18 +128,19 @@ def test_can_create_spreadsheet_from_dict_with_filename():
 
 
 def test_generate_notifications_csv_returns_correct_csv_file(_get_notifications_csv_mock):
-    csv_content = generate_notifications_csv(service_id='1234')
+    csv_content = generate_notifications_csv(service_id='1234', job_id=fake_uuid)
     csv_file = DictReader(StringIO('\n'.join(csv_content)))
     assert csv_file.fieldnames == ['Row number', 'Recipient', 'Template', 'Type', 'Job', 'Status', 'Time']
 
 
 def test_generate_notifications_csv_only_calls_once_if_no_next_link(_get_notifications_csv_mock):
-    list(generate_notifications_csv(service_id='1234'))
+    list(generate_notifications_csv(service_id='1234', job_id=fake_uuid))
 
     assert _get_notifications_csv_mock.call_count == 1
 
 
-def test_generate_notifications_csv_calls_twice_if_next_link(mocker):
+@pytest.mark.parametrize("job_id", ["some", None])
+def test_generate_notifications_csv_calls_twice_if_next_link(mocker, job_id):
     service_id = '1234'
     response_with_links = _get_notifications_csv(service_id, rows=7, with_links=True)
     response_with_no_links = _get_notifications_csv(service_id, rows=3, with_links=False)
@@ -146,7 +153,7 @@ def test_generate_notifications_csv_calls_twice_if_next_link(mocker):
         ]
     )
 
-    csv_content = generate_notifications_csv(service_id=service_id)
+    csv_content = generate_notifications_csv(service_id=service_id, job_id=fake_uuid if job_id else None)
     csv = DictReader(StringIO('\n'.join(csv_content)))
 
     assert len(list(csv)) == 10

tests/conftest.py

@@ -1236,6 +1236,31 @@ def active_user_manage_template_permission(fake_uuid):
     return user
 
 
+@pytest.fixture
+def active_user_no_api_key_permission(fake_uuid):
+    from app.notify_client.user_api_client import User
+
+    user_data = {
+        'id': fake_uuid,
+        'name': 'Test User With Permissions',
+        'password': 'somepassword',
+        'password_changed_at': str(datetime.utcnow()),
+        'email_address': 'test@user.gov.uk',
+        'mobile_number': '07700 900762',
+        'state': 'active',
+        'failed_login_count': 0,
+        'permissions': {SERVICE_ONE_ID: [
+            'manage_templates',
+            'manage_settings',
+            'view_activity',
+        ]},
+        'platform_admin': False,
+        'auth_type': 'sms_auth'
+    }
+    user = User(user_data)
+    return user
+
+
 @pytest.fixture(scope='function')
 def api_user_locked(fake_uuid):
     from app.notify_client.user_api_client import User