diff --git a/app/main/views/jobs.py b/app/main/views/jobs.py index 511716530..8beb59f44 100644 --- a/app/main/views/jobs.py +++ b/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//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['status'] = _set_status_filters(filter_args) + filter_args = parse_filter_args(request.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['status'] = _set_status_filters(filter_args) + filter_args = parse_filter_args(request.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['status'] = _set_status_filters(filter_args) + filter_args = parse_filter_args(request.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['status'] = _set_status_filters(filter_args) + filter_args = parse_filter_args(request.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'] ) diff --git a/app/main/views/notifications.py b/app/main/views/notifications.py index aa7e82ccc..438ad5d50 100644 --- a/app/main/views/notifications.py +++ b/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//notification/") @@ -138,3 +140,31 @@ def get_all_personalisation_from_notification(notification): notification['personalisation']['phone_number'] = notification['to'] return notification['personalisation'] + + +@main.route("/services//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']) + } + ) diff --git a/app/templates/views/api/callbacks.html b/app/templates/views/api/callbacks.html index 54a86bafe..126342b37 100644 --- a/app/templates/views/api/callbacks.html +++ b/app/templates/views/api/callbacks.html @@ -30,4 +30,8 @@ {% endcall %} {% endcall %} + {{ page_footer( + secondary_link=url_for('.api_integration', service_id=current_service.id), + secondary_link_text='Back to API integration' + ) }} {% endblock %} diff --git a/app/templates/views/notifications.html b/app/templates/views/notifications.html index b76641b34..c1eb2985d 100644 --- a/app/templates/views/notifications.html +++ b/app/templates/views/notifications.html @@ -42,8 +42,16 @@ + {% else %} +
+ +
{% endif %} - + + + + + {{ ajax_block( partials, url_for('.get_notifications_as_json', service_id=current_service.id, message_type=message_type, status=status, page=page), diff --git a/app/templates/views/service-settings/name.html b/app/templates/views/service-settings/name.html index 419cd7d03..223d079e9 100644 --- a/app/templates/views/service-settings/name.html +++ b/app/templates/views/service-settings/name.html @@ -10,13 +10,16 @@

Change your service name

-
-

Users will see your service name:

-
    -
  • at the start of every text message, eg ‘Vehicle tax: we received your payment, thank you’
  • -
  • as your email sender name
  • -
+ {% if current_service.prefix_sms %} +

Users will see your service name:

+
    +
  • at the start of every text message, eg ‘{{ current_service.name }}: This is an example message’
  • +
  • as your email sender name
  • +
+ {% else %} +

Users will see your service name as your email sender name.

+ {% endif %}
diff --git a/app/templates/views/service-settings/set-inbound-sms.html b/app/templates/views/service-settings/set-inbound-sms.html index 6fef29a27..bc6459783 100644 --- a/app/templates/views/service-settings/set-inbound-sms.html +++ b/app/templates/views/service-settings/set-inbound-sms.html @@ -19,6 +19,12 @@ If you want to turn this feature off, get in touch with the GOV.UK Notify team.

+ {% if current_user.has_permissions(['manage_api_keys'], admin_override=True) %} +

+ You can set up callbacks for received text messages on the + API integration page. +

+ {% endif %} {% else %}

Receiving text messages from your users is an diff --git a/app/utils.py b/app/utils.py index bbeebb691..b38b7665b 100644 --- a/app/utils.py +++ b/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: - values = [ - notification['row_number'], - notification['recipient'], - notification['template_name'], - notification['template_type'], - notification['job_name'], - notification['status'], - notification['created_at'] - ] - line = ','.join(str(i) for i in values) + '\n' - yield line - + if kwargs['job_id']: + for notification in notifications: + values = [ + notification['row_number'], + notification['recipient'], + notification['template_name'], + notification['template_type'], + notification['job_name'], + notification['status'], + notification['created_at'] + ] + 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 [] + ))) diff --git a/requirements.txt b/requirements.txt index 6bb28a328..e7af49127 100644 --- a/requirements.txt +++ b/requirements.txt @@ -5,8 +5,8 @@ Flask-Login==0.4.1 boto3==1.5.12 blinker==1.4 -pyexcel==0.5.6 -pyexcel-io==0.5.5 +pyexcel==0.5.7 +pyexcel-io==0.5.6 pyexcel-xls==0.5.5 pyexcel-xlsx==0.5.5 pyexcel-ods3==0.5.2 diff --git a/requirements_for_test.txt b/requirements_for_test.txt index 7e0d0651e..5ed459a80 100644 --- a/requirements_for_test.txt +++ b/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 diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py index fb8cb7d76..237fe1e42 100644 --- a/tests/app/main/views/test_service_settings.py +++ b/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, - service_one, + client_request, ): - response = logged_in_client.get(url_for( - 'main.service_name_change', service_id=service_one['id'])) - assert response.status_code == 200 - page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + page = client_request.get('main.service_name_change', service_id=SERVICE_ONE_ID) 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, diff --git a/tests/app/test_utils.py b/tests/app/test_utils.py index 980f7a6c7..6b9f3207b 100644 --- a/tests/app/test_utils.py +++ b/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 diff --git a/tests/conftest.py b/tests/conftest.py index b6bd4030b..67e99d810 100644 --- a/tests/conftest.py +++ b/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