Merge pull request #657 from alphagov/make-activity-consistent

Make activity consistent
This commit is contained in:
Chris Hill-Scott
2016-06-08 12:57:07 +01:00
5 changed files with 161 additions and 255 deletions

View File

@@ -43,17 +43,15 @@ def _parse_filter_args(filter_dict):
def _set_status_filters(filter_args):
all_failure_statuses = ['failed', 'temporary-failure', 'permanent-failure', 'technical-failure']
all_statuses = ['sending', 'delivered'] + all_failure_statuses
if filter_args.get('status'):
if 'failed' in filter_args.get('status'):
filter_args['status'].extend(['temporary-failure', 'permanent-failure', 'technical-failure'])
if 'processed' in filter_args.get('status'):
filter_args['status'] = all_statuses
elif 'failed' in filter_args.get('status'):
filter_args['status'].extend(all_failure_statuses[1:])
else:
# default to everything
filter_args['status'] = ['delivered', 'failed', 'temporary-failure', 'permanent-failure', 'technical-failure']
def _set_template_filters(filter_args):
if not filter_args.get('template_type'):
filter_args['template_type'] = ['email', 'sms']
filter_args['status'] = all_statuses
@main.route("/services/<service_id>/jobs")
@@ -77,20 +75,6 @@ def view_job(service_id, job_id):
version=job['template_version'])['data']
notifications = notification_api_client.get_notifications_for_service(service_id, job_id)
finished = job['status'] == 'finished'
if 'download' in request.args and request.args['download'] == 'csv':
csv_content = generate_notifications_csv(
notification_api_client.get_notifications_for_service(
service_id,
job_id,
page_size=job['notification_count']
)['notifications'])
return csv_content, 200, {
'Content-Type': 'text/csv; charset=utf-8',
'Content-Disposition': 'inline; filename="{} - {}.csv"'.format(
template['name'],
format_datetime_short(job['created_at'])
)
}
return render_template(
'views/jobs/job.html',
notifications=notifications['notifications'],
@@ -113,6 +97,36 @@ def view_job(service_id, job_id):
)
@main.route("/services/<service_id>/jobs/<job_id>.csv")
@login_required
@user_has_permissions('view_activity', admin_override=True)
def view_job_csv(service_id, job_id):
job = job_api_client.get_job(service_id, job_id)['data']
template = service_api_client.get_service_template(
service_id=service_id,
template_id=job['template'],
version=job['template_version']
)['data']
return (
generate_notifications_csv(
notification_api_client.get_notifications_for_service(
service_id,
job_id,
page_size=job['notification_count']
)['notifications']
),
200,
{
'Content-Type': 'text/csv; charset=utf-8',
'Content-Disposition': 'inline; filename="{} - {}.csv"'.format(
template['name'],
format_datetime_short(job['created_at'])
)
}
)
@main.route("/services/<service_id>/jobs/<job_id>.json")
@login_required
@user_has_permissions('view_activity', admin_override=True)
@@ -144,26 +158,31 @@ def view_job_updates(service_id, job_id):
})
@main.route('/services/<service_id>/notifications')
@main.route('/services/<service_id>/notifications/<message_type>')
@main.route('/services/<service_id>/notifications/<message_type>.csv', endpoint="view_notifications_csv")
@login_required
@user_has_permissions('view_activity', admin_override=True)
def view_notifications(service_id):
def view_notifications(service_id, message_type):
# TODO get the api to return count of pages as well.
page = get_page_from_request()
if page is None:
abort(404, "Invalid page argument ({}) reverting to page 1.".format(request.args['page'], None))
if message_type not in ['email', 'sms']:
abort(404)
filter_args = _parse_filter_args(request.args)
_set_status_filters(filter_args)
_set_template_filters(filter_args)
notifications = notification_api_client.get_notifications_for_service(
service_id=service_id,
page=page,
template_type=filter_args.get('template_type'),
template_type=[message_type],
status=filter_args.get('status'),
limit_days=current_app.config['ACTIVITY_STATS_LIMIT_DAYS'])
view_dict = MultiDict(request.args)
view_dict = dict(
message_type=message_type,
status=request.args.get('status')
)
prev_page = None
if notifications['links'].get('prev', None):
prev_page = generate_previous_next_dict(
@@ -182,13 +201,13 @@ def view_notifications(service_id):
page + 1,
'Next page',
'page {}'.format(page + 1))
if 'download' in request.args and request.args['download'] == 'csv':
if request.path.endswith('csv'):
csv_content = generate_notifications_csv(
notification_api_client.get_notifications_for_service(
service_id=service_id,
page=page,
page_size=notifications['total'],
template_type=filter_args.get('template_type') if 'template_type' in filter_args else ['email', 'sms'],
template_type=[message_type],
status=filter_args.get('status'),
limit_days=current_app.config['ACTIVITY_STATS_LIMIT_DAYS'])['notifications'])
return csv_content, 200, {
@@ -202,28 +221,24 @@ def view_notifications(service_id):
prev_page=prev_page,
next_page=next_page,
request_args=request.args,
type_filters=[
[item[0], item[1], url_for(
'.view_notifications',
service_id=current_service['id'],
template_type=item[1],
status=request.args.get('status', 'delivered,failed')
)] for item in [
['Emails', 'email'],
['Text messages', 'sms'],
['Both', 'email,sms']
]
],
message_type=message_type,
download_link=url_for(
'.view_notifications_csv',
service_id=current_service['id'],
message_type=message_type,
status=request.args.get('status')
),
status_filters=[
[item[0], item[1], url_for(
'.view_notifications',
service_id=current_service['id'],
template_type=request.args.get('template_type', 'email,sms'),
message_type=message_type,
status=item[1]
)] for item in [
['Successful', 'delivered'],
['Processed', 'sending,delivered,failed'],
['Sending', 'sending'],
['Delivered', 'delivered'],
['Failed', 'failed'],
['Both', 'delivered,failed']
]
]
)

View File

@@ -18,8 +18,8 @@
statistics.emails_failed,
statistics.get('emails_failure_rate', 0.0),
statistics.get('emails_failure_rate', 0)|float > 3,
failure_link=url_for(".view_notifications", service_id=current_service.id, template_type='email', status='failed'),
label_link=url_for(".view_notifications", service_id=current_service.id, template_type='email', status='delivered,failed')
failure_link=url_for(".view_notifications", service_id=current_service.id, message_type='email', status='failed'),
label_link=url_for(".view_notifications", service_id=current_service.id, message_type='email', status='sending,delivered,failed')
) }}
</div>
<div class="column-half">
@@ -29,8 +29,8 @@
statistics.sms_failed,
statistics.get('sms_failure_rate', 0.0),
statistics.get('sms_failure_rate', 0)|float > 3,
failure_link=url_for(".view_notifications", service_id=current_service.id, template_type='sms', status='failed'),
label_link=url_for(".view_notifications", service_id=current_service.id, template_type='sms', status='delivered,failed')
failure_link=url_for(".view_notifications", service_id=current_service.id, message_type='sms', status='failed'),
label_link=url_for(".view_notifications", service_id=current_service.id, message_type='sms', status='sending,delivered,failed')
) }}
</div>
<div class="column-whole">

View File

@@ -3,65 +3,39 @@
{% from "components/previous-next-navigation.html" import previous_next_navigation %}
{% from "components/page-footer.html" import page_footer %}
{% from "components/pill.html" import pill %}
{% from "components/message-count-label.html" import message_count_label %}
{% block page_title %}
Activity GOV.UK Notify
{{ message_count_label(99, message_type, suffix='') | capitalize }} GOV.UK Notify
{% endblock %}
{% block maincolumn_content %}
<h1 class="heading-large">
{%- if (request_args.get('template_type', 'email,sms') == 'email,sms') and (request_args.get('status', 'delivered,failed') == 'delivered,failed') -%}
Activity
{%- else -%}
<span class="visually-hidden">
{%- if request_args.get('status') != 'delivered,failed' -%}
{%- for label, option, _ in status_filters -%}
{%- if request_args.get('status', 'delivered,failed') == option -%}{{label}} {% endif -%}
{%- endfor -%}
{%- endif -%}
</span>
{%- if request_args.get('template_type', 'email,sms') == 'email,sms' %} emails and text messages
{%- else -%}
{%- for template_label, template_option, _ in type_filters -%}
{%- if request_args.get('template_type') == template_option -%}
{%- if request_args.get('status', 'delivered,failed') == 'delivered,failed' -%}
{{ template_label }}
{%- else -%}
{{ template_label | lower }}
{%- endif -%}
{%- endif -%}
{%- endfor -%}
{%- endif -%}
{%- endif -%}
{{- message_count_label(99, message_type, suffix='') | capitalize }}
</h1>
<div class='grid-row bottom-gutter'>
<div class='column-half'>
{{ pill(
'Status',
status_filters,
request_args.get('status', '')
) }}
</div>
<div class='column-half'>
{{ pill(
'Type',
type_filters,
request_args.get('template_type', '')
) }}
</div>
<div class='bottom-gutter'>
{{ pill(
'Status',
status_filters,
request_args.get('status', '')
) }}
</div>
{% if notifications %}
<p class="bottom-gutter">
<a href="{{ request.url }}&amp;download=csv" download class="heading-small">Download as a CSV file</a>
<a href="{{ download_link }}" download="download" class="heading-small">Download as a CSV file</a>
&emsp;
Delivery information is available for 7 days
</p>

View File

@@ -175,7 +175,6 @@ def test_menu_send_messages(mocker,
'main.choose_template',
service_id=service_one['id'],
template_type='sms')in page
assert url_for('main.view_notifications', service_id=service_one['id']) in page
assert url_for('main.manage_users', service_id=service_one['id']) in page
assert url_for('main.documentation') in page
@@ -209,7 +208,6 @@ def test_menu_manage_service(mocker,
'main.choose_template',
service_id=service_one['id'],
template_type='sms') in page
assert url_for('main.view_notifications', service_id=service_one['id']) in page
assert url_for('main.manage_users', service_id=service_one['id']) in page
assert url_for('main.service_settings', service_id=service_one['id']) in page
assert url_for('main.documentation') in page
@@ -242,7 +240,6 @@ def test_menu_manage_api_keys(mocker,
'main.choose_template',
service_id=service_one['id'],
template_type='sms') in page
assert url_for('main.view_notifications', service_id=service_one['id']) in page
assert url_for('main.manage_users', service_id=service_one['id']) in page
assert url_for('main.service_settings', service_id=service_one['id']) not in page
assert url_for('main.show_all_services') not in page
@@ -270,7 +267,8 @@ def test_menu_all_services_for_platform_admin_user(mocker,
assert url_for('main.choose_template', service_id=service_one['id'], template_type='email') in page
assert url_for('main.manage_users', service_id=service_one['id']) in page
assert url_for('main.service_settings', service_id=service_one['id']) in page
assert url_for('main.view_notifications', service_id=service_one['id']) in page
assert url_for('main.view_notifications', service_id=service_one['id'], message_type='email') in page
assert url_for('main.view_notifications', service_id=service_one['id'], message_type='sms') in page
assert url_for('main.api_keys', service_id=service_one['id']) not in page
# Should this be here??

View File

@@ -1,3 +1,4 @@
import pytest
from flask import url_for
from bs4 import BeautifulSoup
import json
@@ -78,143 +79,42 @@ def test_should_show_updates_for_one_job_as_json(
assert 'Uploaded by Test User on 1 January at 11:09' in content['status']
def test_should_show_notifications_for_a_service(app_,
service_one,
active_user_with_permissions,
mock_get_notifications,
mocker):
with app_.test_request_context():
with app_.test_client() as client:
client.login(active_user_with_permissions, mocker, service_one)
response = client.get(url_for('main.view_notifications', service_id=service_one['id']))
assert response.status_code == 200
content = response.get_data(as_text=True)
notifications = notification_json(service_one['id'])
notification = notifications['notifications'][0]
assert notification['to'] in content
assert notification['status'] in content
assert notification['template']['name'] in content
assert 'csv' in content
page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
assert page.h1.string.strip() == 'Activity'
mock_get_notifications.assert_called_with(limit_days=7, page=1, service_id=service_one['id'], status=['delivered', 'failed', 'temporary-failure', 'permanent-failure', 'technical-failure'], template_type=['email', 'sms']) # noqa
def test_can_view_only_sms_notifications_for_a_service(app_,
service_one,
active_user_with_permissions,
mock_get_notifications,
mocker):
with app_.test_request_context():
with app_.test_client() as client:
client.login(active_user_with_permissions, mocker, service_one)
response = client.get(url_for(
'main.view_notifications',
service_id=service_one['id'],
template_type='sms',
status='delivered,failed'))
assert response.status_code == 200
content = response.get_data(as_text=True)
notifications = notification_json(service_one['id'])
notification = notifications['notifications'][0]
assert notification['to'] in content
assert notification['status'] in content
assert notification['template']['name'] in content
assert 'csv' in content
page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
assert page.h1.string.strip() == 'Text messages'
mock_get_notifications.assert_called_with(limit_days=7, page=1, service_id=service_one['id'], status=['delivered', 'failed', 'temporary-failure', 'permanent-failure', 'technical-failure'], template_type=['sms']) # noqa
def test_can_view_only_email_notifications_for_a_service(app_,
service_one,
active_user_with_permissions,
mock_get_notifications,
mocker):
with app_.test_request_context():
with app_.test_client() as client:
client.login(active_user_with_permissions, mocker, service_one)
response = client.get(url_for(
'main.view_notifications',
service_id=service_one['id'],
status='delivered,failed',
template_type='email'))
assert response.status_code == 200
content = response.get_data(as_text=True)
notifications = notification_json(service_one['id'])
notification = notifications['notifications'][0]
assert notification['to'] in content
assert notification['status'] in content
assert notification['template']['name'] in content
assert 'csv' in content
page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
assert page.h1.string.strip() == 'Emails'
mock_get_notifications.assert_called_with(limit_days=7, page=1, service_id=service_one['id'], status=['delivered', 'failed', 'temporary-failure', 'permanent-failure', 'technical-failure'], template_type=['email']) # noqa
def test_can_view_successful_notifications_for_a_service(app_,
service_one,
active_user_with_permissions,
mock_get_notifications,
mocker):
with app_.test_request_context():
with app_.test_client() as client:
client.login(active_user_with_permissions, mocker, service_one)
response = client.get(url_for(
'main.view_notifications',
service_id=service_one['id'],
status='delivered'))
assert response.status_code == 200
content = response.get_data(as_text=True)
notifications = notification_json(service_one['id'])
notification = notifications['notifications'][0]
assert notification['to'] in content
assert notification['status'] in content
assert notification['template']['name'] in content
assert 'csv' in content
page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
assert page.h1.string.strip() == 'Successful emails and text messages'
mock_get_notifications.assert_called_with(limit_days=7, page=1, service_id=service_one['id'], status=['delivered'], template_type=['email', 'sms']) # noqa
def test_can_view_failed_notifications_for_a_service(app_,
service_one,
active_user_with_permissions,
mock_get_notifications,
mocker):
with app_.test_request_context():
with app_.test_client() as client:
client.login(active_user_with_permissions, mocker, service_one)
response = client.get(url_for(
'main.view_notifications',
service_id=service_one['id'],
status='failed'))
assert response.status_code == 200
content = response.get_data(as_text=True)
notifications = notification_json(service_one['id'])
notification = notifications['notifications'][0]
assert notification['to'] in content
assert notification['status'] in content
assert notification['template']['name'] in content
assert 'csv' in content
page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
assert page.h1.string.strip() == 'Failed emails and text messages'
mock_get_notifications.assert_called_with(limit_days=7, page=1, service_id=service_one['id'], status=['failed', 'temporary-failure', 'permanent-failure', 'technical-failure'], template_type=['email', 'sms']) # noqa
def test_can_view_failed_combination_of_notification_type_and_status(
@pytest.mark.parametrize(
"message_type,page_title", [
('email', 'Emails'),
('sms', 'Text messages')
]
)
@pytest.mark.parametrize(
"status_argument, expected_api_call", [
(
'processed',
['sending', 'delivered', 'failed', 'temporary-failure', 'permanent-failure', 'technical-failure']
),
(
'sending',
['sending']
),
(
'delivered',
['delivered']
),
(
'failed',
['failed', 'temporary-failure', 'permanent-failure', 'technical-failure']
)
]
)
def test_can_show_notifications(
app_,
service_one,
active_user_with_permissions,
mock_get_notifications,
mocker
mocker,
message_type,
page_title,
status_argument,
expected_api_call
):
with app_.test_request_context():
with app_.test_client() as client:
@@ -222,13 +122,46 @@ def test_can_view_failed_combination_of_notification_type_and_status(
response = client.get(url_for(
'main.view_notifications',
service_id=service_one['id'],
status='failed',
template_type='sms'))
message_type=message_type,
status=status_argument))
assert response.status_code == 200
page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
assert page.h1.string.strip() == 'Failed text messages'
content = response.get_data(as_text=True)
mock_get_notifications.assert_called_with(limit_days=7, page=1, service_id=service_one['id'], status=['failed', 'temporary-failure', 'permanent-failure', 'technical-failure'], template_type=['sms']) # noqa
notifications = notification_json(service_one['id'])
notification = notifications['notifications'][0]
assert notification['to'] in content
assert notification['status'] in content
assert notification['template']['name'] in content
assert 'csv' in content
page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
assert page_title in page.h1.text.strip()
assert url_for(
'.view_notifications_csv',
service_id=service_one['id'],
message_type=message_type,
status=status_argument
) == page.findAll("a", {"download": "download"})[0]['href']
mock_get_notifications.assert_called_with(
limit_days=7,
page=1,
service_id=service_one['id'],
status=expected_api_call,
template_type=[message_type]
)
csv_response = client.get(url_for(
'main.view_notifications_csv',
service_id=service_one['id'],
message_type='email',
download='csv'
))
csv_content = generate_notifications_csv(
mock_get_notifications(service_one['id'])['notifications']
)
assert csv_response.status_code == 200
assert csv_response.get_data(as_text=True) == csv_content
assert 'text/csv' in csv_response.headers['Content-Type']
def test_should_show_notifications_for_a_service_with_next_previous(app_,
@@ -236,36 +169,21 @@ def test_should_show_notifications_for_a_service_with_next_previous(app_,
active_user_with_permissions,
mock_get_notifications_with_previous_next,
mocker):
with app_.test_request_context():
with app_.test_client() as client:
client.login(active_user_with_permissions, mocker, service_one)
response = client.get(url_for('main.view_notifications', service_id=service_one['id'], page=2))
assert response.status_code == 200
content = response.get_data(as_text=True)
assert url_for('main.view_notifications', service_id=service_one['id'], page=3) in content
assert url_for('main.view_notifications', service_id=service_one['id'], page=1) in content
assert 'Previous page' in content
assert 'Next page' in content
def test_should_download_notifications_for_a_service(app_,
service_one,
active_user_with_permissions,
mock_get_service_template,
mock_get_notifications,
mocker):
with app_.test_request_context():
with app_.test_client() as client:
client.login(active_user_with_permissions, mocker, service_one)
response = client.get(url_for(
'main.view_notifications',
service_id=service_one['id'],
download='csv'))
csv_content = generate_notifications_csv(
mock_get_notifications(service_one['id'])['notifications'])
message_type='sms',
page=2
))
assert response.status_code == 200
assert response.get_data(as_text=True) == csv_content
assert 'text/csv' in response.headers['Content-Type']
content = response.get_data(as_text=True)
assert url_for('main.view_notifications', service_id=service_one['id'], message_type='sms', page=3) in content
assert url_for('main.view_notifications', service_id=service_one['id'], message_type='sms', page=1) in content
assert 'Previous page' in content
assert 'Next page' in content
@freeze_time("2016-01-01 11:09:00.061258")
@@ -282,12 +200,13 @@ def test_should_download_notifications_for_a_job(app_,
with app_.test_client() as client:
client.login(api_user_active)
response = client.get(url_for(
'main.view_job',
'main.view_job_csv',
service_id=fake_uuid,
job_id=fake_uuid,
download='csv'))
))
csv_content = generate_notifications_csv(
mock_get_notifications(fake_uuid, job_id=fake_uuid)['notifications'])
mock_get_notifications(fake_uuid, job_id=fake_uuid)['notifications']
)
assert response.status_code == 200
assert response.get_data(as_text=True) == csv_content
assert 'text/csv' in response.headers['Content-Type']