Revert "Merge pull request #1336 from alphagov/revert-show-notifications"

This reverts commit 7e354ff341, reversing
changes made to 6f3bcff32f.
This commit is contained in:
Chris Hill-Scott
2017-06-24 17:12:45 +01:00
parent 8115dab8e4
commit 9f20ea4b7e
21 changed files with 223 additions and 196 deletions

View File

@@ -66,6 +66,7 @@ $path: '/static/images/';
@import 'views/api';
@import 'views/product-page';
@import 'views/template';
@import 'views/notification';
// TODO: break this up
@import 'app';

View File

@@ -46,17 +46,6 @@
transition: width 0.6s ease-in-out;
}
&-label {
@include bold-19;
display: block;
white-space: nowrap;
overflow: hidden;
text-overflow: ellipsis;
color: $govuk-blue;
max-width: 100%;
background: $white;
}
}
.file-list {
@@ -67,12 +56,17 @@
white-space: nowrap;
overflow: hidden;
text-overflow: ellipsis;
padding-bottom: 30px;
padding-top: 10px;
margin-bottom: -30px;
margin-top: -10px;
}
&-hint {
@include core-16;
display: block;
color: $secondary-text-colour;
pointer-events: none;
}
}
@@ -96,5 +90,6 @@
}
.align-with-message-body {
display: block;
margin-top: $gutter * 5 / 6;
}

View File

@@ -0,0 +1,18 @@
.notification-status {
@include core-16;
color: $secondary-text-colour;
margin-top: -$gutter-half;
&.error {
color: $error-colour;
font-weight: bold;
a {
color: $error-colour;
}
}
}

View File

@@ -14,6 +14,10 @@ from flask import (
stream_with_context
)
from flask_login import login_required
from notifications_utils.template import (
Template,
WithSubjectTemplate,
)
from werkzeug.datastructures import MultiDict
from app import (
@@ -110,21 +114,7 @@ def view_job(service_id, job_id):
'views/jobs/job.html',
finished=(total_notifications == processed_notifications),
uploaded_file_name=job['original_file_name'],
template=get_template(
service_api_client.get_service_template(
service_id=service_id,
template_id=job['template'],
version=job['template_version']
)['data'],
current_service,
letter_preview_url=url_for(
'.view_template_version_preview',
service_id=service_id,
template_id=job['template'],
version=job['template_version'],
filetype='png',
),
),
template_id=job['template'],
status=request.args.get('status', ''),
updates_url=url_for(
".view_job_updates",
@@ -271,7 +261,7 @@ def get_notifications(service_id, message_type, status_override=None):
),
'notifications': render_template(
'views/activity/notifications.html',
notifications=notifications['notifications'],
notifications=add_preview_of_content_to_notifications(notifications['notifications']),
page=page,
prev_page=prev_page,
next_page=next_page,
@@ -360,6 +350,11 @@ def get_job_partials(job):
notifications = notification_api_client.get_notifications_for_service(
job['service'], job['id'], status=filter_args['status']
)
template = service_api_client.get_service_template(
service_id=current_service['id'],
template_id=job['template'],
version=job['template_version']
)['data']
return {
'counts': render_template(
'partials/count.html',
@@ -368,7 +363,9 @@ def get_job_partials(job):
),
'notifications': render_template(
'partials/jobs/notifications.html',
notifications=notifications['notifications'],
notifications=list(
add_preview_of_content_to_notifications(notifications['notifications'])
),
more_than_one_page=bool(notifications.get('links', {}).get('next')),
percentage_complete=(job['notifications_requested'] / job['notification_count'] * 100),
download_link=url_for(
@@ -379,10 +376,26 @@ def get_job_partials(job):
),
help=get_help_argument(),
time_left=get_time_left(job['created_at']),
job=job
job=job,
template=template,
template_version=job['template_version'],
),
'status': render_template(
'partials/jobs/status.html',
job=job
),
}
def add_preview_of_content_to_notifications(notifications):
return [
dict(
preview_of_content=(
str(Template(notification['template'], notification['personalisation']))
if notification['template']['template_type'] == 'sms' else
WithSubjectTemplate(notification['template'], notification['personalisation']).subject
),
**notification
)
for notification in notifications
]

View File

@@ -10,6 +10,7 @@ from flask_login import login_required
from app import (
notification_api_client,
job_api_client,
current_service
)
from app.main import main
@@ -39,27 +40,34 @@ def get_status_arg(filter_args):
return REQUESTED_STATUSES
@main.route("/services/<service_id>/one-off-notification/<notification_id>")
@main.route("/services/<service_id>/notification/<notification_id>")
@login_required
@user_has_permissions('view_activity', admin_override=True)
def view_notification(service_id, notification_id):
notification = notification_api_client.get_notification(service_id, notification_id)
template = get_template(
notification['template'],
current_service,
letter_preview_url=url_for(
'.view_template_version_preview',
service_id=service_id,
template_id=notification['template']['id'],
version=notification['template_version'],
filetype='png',
),
show_recipient=True,
)
template.values = get_all_personalisation_from_notification(notification)
if notification['job']:
job = job_api_client.get_job(service_id, notification['job']['id'])['data']
else:
job = None
return render_template(
'views/notifications/notification.html',
finished=(notification['status'] in (DELIVERED_STATUSES + FAILURE_STATUSES)),
uploaded_file_name='Report',
template=get_template(
notification['template'],
current_service,
letter_preview_url=url_for(
'.view_template_version_preview',
service_id=service_id,
template_id=notification['template']['id'],
version=notification['template_version'],
filetype='png',
),
),
status=request.args.get('status'),
template=template,
job=job,
updates_url=url_for(
".view_notification_updates",
service_id=service_id,
@@ -68,11 +76,13 @@ def view_notification(service_id, notification_id):
help=get_help_argument()
),
partials=get_single_notification_partials(notification),
created_by=notification.get('created_by'),
created_at=notification['created_at'],
help=get_help_argument()
)
@main.route("/services/<service_id>/one-off-notification/<notification_id>.json")
@main.route("/services/<service_id>/notification/<notification_id>.json")
@user_has_permissions('view_activity', admin_override=True)
def view_notification_updates(service_id, notification_id):
return jsonify(**get_single_notification_partials(
@@ -80,49 +90,10 @@ def view_notification_updates(service_id, notification_id):
))
def _get_single_notification_counts(notification, help_argument):
return [
(
label,
query_param,
url_for(
".view_notification",
service_id=notification['service'],
notification_id=notification['id'],
status=query_param,
help=help_argument
),
count
) for label, query_param, count in [
[
'total', '',
1
],
[
'sending', 'sending',
int(notification['status'] in SENDING_STATUSES)
],
[
'delivered', 'delivered',
int(notification['status'] in DELIVERED_STATUSES)
],
[
'failed', 'failed',
int(notification['status'] in FAILURE_STATUSES)
]
]
]
def get_single_notification_partials(notification):
status_args = get_status_arg(request.args)
return {
'counts': render_template(
'partials/count.html',
counts=_get_single_notification_counts(notification, request.args.get('help', 0)),
status=status_args
),
'notifications': render_template(
'partials/notifications/notifications.html',
notification=notification,
@@ -135,3 +106,18 @@ def get_single_notification_partials(notification):
notification=notification
),
}
def get_all_personalisation_from_notification(notification):
if notification['template']['template_type'] == 'email':
return dict(
email_address=notification['to'],
**notification['personalisation']
)
if notification['template']['template_type'] == 'sms':
return dict(
phone_number=notification['to'],
**notification['personalisation']
)
if notification['template']['template_type'] == 'letter':
return notification['personalisation']

View File

@@ -120,6 +120,7 @@
{% macro notification_status_field(notification) %}
{% call field(status=notification.status|format_notification_status_as_field_status, align='right') %}
{% if notification.status in ['created', 'sending', 'delivered'] %}<span class="align-with-message-body">{% endif %}
{% if notification.status|format_notification_status_as_url %}
<a href="{{ notification.status|format_notification_status_as_url }}">
{% endif %}
@@ -135,6 +136,7 @@
(notification.updated_at or notification.created_at)|format_datetime_short
) }}
</span>
{% if notification.status in ['created', 'sending', 'delivered'] %}</span>{% endif %}
{% endcall %}
{% endmacro %}

View File

@@ -32,7 +32,7 @@
Notify delivers the message
</p>
{% if help == '3' %}
<a href='{{ url_for(".go_to_dashboard_after_tour", service_id=current_service.id, example_template_id=template.id) }}'>
<a href='{{ url_for(".go_to_dashboard_after_tour", service_id=current_service.id, example_template_id=template_id) }}'>
Now go to your dashboard
</a>
{% endif %}

View File

@@ -5,7 +5,9 @@
{% if job.job_status == 'scheduled' %}
<p>
Sending will start {{ job.scheduled_for|format_datetime_relative }}
Sending
<a href="{{ url_for('.view_template_version', service_id=current_service.id, template_id=template.id, version=template_version) }}">{{ template.name }}</a>
{{ job.scheduled_for|format_datetime_relative }}
</p>
<div class="page-footer">
<form method="post">
@@ -48,7 +50,10 @@
field_headings_visible=False
) %}
{% call row_heading() %}
<p>{{ item.to }}</p>
<a class="file-list-filename" href="{{ url_for('.view_notification', service_id=current_service.id, notification_id=item.id) }}">{{ item.to }}</a>
<p class="file-list-hint">
{{ item.preview_of_content }}
</p>
{% endcall %}
{{ notification_status_field(item) }}
{% endcall %}

View File

@@ -1,5 +1,5 @@
<div class="ajax-block-container">
<p class='heading-small bottom-gutter'>
<p class='bottom-gutter'>
{% if job.scheduled_for %}
{% if job.processing_started %}
Sent by {{ job.created_by.name }} on {{ job.processing_started|format_datetime_short }}

View File

@@ -1,6 +1,13 @@
<div class="ajax-block-container">
<p class='heading-small bottom-gutter'>
Sent {% if notification.created_by %}by {{ notification.created_by.name }} {% endif %}
on {{ notification.created_at|format_datetime_short }}
<p class="notification-status {{ notification.status|format_notification_status_as_field_status }}">
{% if notification.status|format_notification_status_as_url %}
<a href="{{ notification.status|format_notification_status_as_url }}">
{% endif %}
{{ notification.status|format_notification_status(
notification.template.template_type
) }}
{% if notification.status|format_notification_status_as_url %}
</a>
{% endif %}
</p>
</div>

View File

@@ -17,19 +17,9 @@
) %}
{% call row_heading() %}
<p>
{{ item.to }}
</p>
<p class="hint">
{% if item.job and item.job.original_file_name == 'Report' %}
<a href="{{ url_for('.view_template_version', service_id=current_service.id, template_id=item.template.id, version=item.template_version) }}">{{ item.template.name }}</a>
sent to one recipient
{% elif item.job %}
From <a href="{{ url_for(".view_job", service_id=current_service.id, job_id=item.job.id) }}">{{ item.job.original_file_name }}</a>
{% else %}
<a href="{{ url_for('.view_template_version', service_id=current_service.id, template_id=item.template.id, version=item.template_version) }}">{{ item.template.name }}</a>
from an API call
{% endif %}
<a class="file-list-filename" href="{{ url_for('.view_notification', service_id=current_service.id, notification_id=item.id) }}">{{ item.to }}</a>
<p class="file-list-hint">
{{ item.preview_of_content }}
</p>
{% endcall %}

View File

@@ -41,11 +41,9 @@
field_headings_visible=False
) %}
{% call row_heading() %}
<span class="spark-bar-label">
<a href="{{ url_for('.view_template', service_id=current_service.id, template_id=item.id) }}">{{ item.name }}</a>
<span class="file-list-hint">
{{ message_count_label(1, item.type, suffix='template')|capitalize }}
</span>
<a class="file-list-filename" href="{{ url_for('.view_template', service_id=current_service.id, template_id=item.id) }}">{{ item.name }}</a>
<span class="file-list-hint">
{{ message_count_label(1, item.type, suffix='template')|capitalize }}
</span>
{% endcall %}
{{ spark_bar_field(item.requested_count, most_used_template_count) }}

View File

@@ -31,10 +31,10 @@
>
{{ item.user_number | format_phone_number_human_readable }}
</a>
<span class="wide-left-hand-column">{{ item.content }}</span>
<span class="file-list-hint">{{ item.content }}</span>
{% endcall %}
{% call field(align='right') %}
<span class="file-list-hint align-with-message-body">
<span class="align-with-message-body">
{{ item.created_at | format_delta }}
</span>
{% endcall %}

View File

@@ -17,11 +17,9 @@
) %}
{% call row_heading() %}
<span class="spark-bar-label">
<a href="{{ url_for('.view_template', service_id=current_service.id, template_id=item.template_id) }}">{{ item.template_name }}</a>
<span class="file-list-hint">
{{ message_count_label(1, item.template_type, suffix='template')|capitalize }}
</span>
<a class="file-list-filename" href="{{ url_for('.view_template', service_id=current_service.id, template_id=item.template_id) }}">{{ item.template_name }}</a>
<span class="file-list-hint">
{{ message_count_label(1, item.template_type, suffix='template')|capitalize }}
</span>
{% endcall %}
{% if template_statistics|length > 1 %}

View File

@@ -13,15 +13,8 @@
{{ uploaded_file_name }}
</h1>
{{ template|string }}
{{ ajax_block(partials, updates_url, 'status', finished=finished) }}
{{ ajax_block(partials, updates_url, 'counts', finished=finished) }}
{{ ajax_block(partials, updates_url, 'notifications', finished=finished) }}
{{ page_footer(
secondary_link=url_for('.view_template', service_id=current_service.id, template_id=template.id),
secondary_link_text='Back to {}'.format(template.name)
) }}
{% endblock %}

View File

@@ -1,6 +1,7 @@
{% extends "withnav_template.html" %}
{% from "components/banner.html" import banner %}
{% from "components/ajax-block.html" import ajax_block %}
{% from "components/message-count-label.html" import message_count_label %}
{% from "components/page-footer.html" import page_footer %}
{% block service_page_title %}
@@ -10,20 +11,23 @@
{% block maincolumn_content %}
<h1 class="heading-large">
Report
{{ message_count_label(1, template.template_type, suffix='') | capitalize }}
</h1>
<p>
<a href="{{ url_for('.view_template', service_id=current_service.id, template_id=template.id) }}">{{ template.name }}</a>
sent
{% if job and job.original_file_name != 'Report' %}
from
<a href="{{ url_for('.view_job', service_id=current_service.id, job_id=job.id) }}">{{ job.original_file_name }}</a>
{% elif created_by %}
by {{ created_by.name }}
{% endif %}
on {{ created_at|format_datetime_short }}
</p>
{{ template|string }}
{{ ajax_block(partials, updates_url, 'status', finished=finished) }}
{{ ajax_block(partials, updates_url, 'counts', finished=finished) }}
{{ ajax_block(partials, updates_url, 'notifications', finished=finished) }}
{% if not help %}
{{ page_footer(
secondary_link=url_for('.view_template', service_id=current_service.id, template_id=template.id),
secondary_link_text='Back to {}'.format(template.name)
) }}
{% endif %}
{% endblock %}

View File

@@ -105,6 +105,8 @@ def template_json(service_id,
'archived': archived,
'process_type': process_type,
}
if subject is None and type_ != 'sms':
template['subject'] = "template subject"
if subject is not None:
template['subject'] = subject
if redact_personalisation is not None:
@@ -184,7 +186,7 @@ def job_json(
if job_id is None:
job_id = str(generate_uuid())
if template_id is None:
template_id = str(generate_uuid())
template_id = "5d729fbd-239c-44ab-b498-75a985f3198f"
if created_at is None:
created_at = str(datetime.now(timezone.utc).strftime('%Y-%m-%dT%H:%M:%S.%f%z'))
data = {
@@ -220,7 +222,8 @@ def notification_json(
created_at=None,
updated_at=None,
with_links=False,
rows=5
rows=5,
personalisation=None,
):
if template is None:
template = template_json(service_id, str(generate_uuid()))
@@ -259,7 +262,8 @@ def notification_json(
'updated_at': updated_at,
'job_row_number': job_row_number,
'service': service_id,
'template_version': template['version']
'template_version': template['version'],
'personalisation': personalisation or {},
} for i in range(rows)],
'total': rows,
'page_size': 50,

View File

@@ -9,6 +9,7 @@ from bs4 import BeautifulSoup
from app.main.views.jobs import get_time_left, get_status_filters
from tests import notification_json
from tests.conftest import SERVICE_ONE_ID
from tests.app.test_utils import normalize_spaces
from freezegun import freeze_time
@@ -92,13 +93,17 @@ def test_can_show_notifications(
page=page_argument,
))
assert response.status_code == 200
page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
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
page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
text_of_first_row = page.select('tbody tr')[0].text
assert '07123456789' in text_of_first_row
assert (
'template content' in text_of_first_row or
'template subject' in text_of_first_row
)
assert 'Delivered' in text_of_first_row
assert page_title in page.h1.text.strip()
path_to_json = page.find("div", {'data-key': 'notifications'})['data-resource']
@@ -131,6 +136,23 @@ def test_can_show_notifications(
assert json_content.keys() == {'counts', 'notifications'}
def test_shows_message_when_no_notifications(
client_request,
mock_get_detailed_service,
mock_get_notifications_with_no_notifications,
):
page = client_request.get(
'main.view_notifications',
service_id=SERVICE_ONE_ID,
message_type='sms',
)
assert normalize_spaces(page.select('tbody tr')[0].text) == (
'No messages found'
)
@pytest.mark.parametrize((
'initial_query_arguments,'
'form_post_data,'

View File

@@ -8,6 +8,7 @@ from bs4 import BeautifulSoup
from app.main.views.jobs import get_time_left, get_status_filters
from tests import notification_json
from tests.app.test_utils import normalize_spaces
from tests.conftest import SERVICE_ONE_ID
from freezegun import freeze_time
@@ -80,6 +81,7 @@ def test_should_show_page_for_one_job(
status_argument,
expected_api_call,
):
response = logged_in_client.get(url_for(
'main.view_job',
service_id=service_one['id'],
@@ -90,11 +92,8 @@ def test_should_show_page_for_one_job(
assert response.status_code == 200
page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
assert page.h1.text.strip() == 'thisisatest.csv'
assert page.find('div', {'class': 'sms-message-wrapper'}).text.strip() == (
'{}: Template <em>content</em> with & entity'.format(service_one['name'])
)
assert ' '.join(page.find('tbody').find('tr').text.split()) == (
'07123456789 Delivered 1 January at 11:10am'
'07123456789 template content Delivered 1 January at 11:10am'
)
assert page.find('div', {'data-key': 'notifications'})['data-resource'] == url_for(
'main.view_job_updates',
@@ -163,7 +162,6 @@ def test_should_show_job_in_progress(
@freeze_time("2016-01-01T00:00:00.061258")
def test_should_show_scheduled_job(
logged_in_client,
service_one,
active_user_with_permissions,
mock_get_service_template,
mock_get_scheduled_job,
@@ -173,13 +171,21 @@ def test_should_show_scheduled_job(
):
response = logged_in_client.get(url_for(
'main.view_job',
service_id=service_one['id'],
service_id=SERVICE_ONE_ID,
job_id=fake_uuid
))
assert response.status_code == 200
page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
assert page.find('main').find_all('p')[1].text.strip() == 'Sending will start today at midnight'
assert normalize_spaces(page.select('main p')[1].text) == (
'Sending Two week reminder today at midnight'
)
assert page.select('main p a')[0]['href'] == url_for(
'main.view_template_version',
service_id=SERVICE_ONE_ID,
template_id='5d729fbd-239c-44ab-b498-75a985f3198f',
version=1,
)
assert page.find('input', {'type': 'submit', 'value': 'Cancel sending'})
@@ -256,6 +262,7 @@ def test_should_show_updates_for_one_job_as_json(
service_one,
active_user_with_permissions,
mock_get_notifications,
mock_get_service_template,
mock_get_job,
mocker,
fake_uuid,

View File

@@ -1,8 +1,6 @@
from freezegun import freeze_time
import pytest
from werkzeug.datastructures import MultiDict
from app.main.views.notifications import get_status_arg
from app.utils import (
REQUESTED_STATUSES,
FAILURE_STATUSES,
@@ -10,63 +8,34 @@ from app.utils import (
DELIVERED_STATUSES,
)
from tests.app.test_utils import normalize_spaces
from tests.conftest import mock_get_notification
@pytest.mark.parametrize('multidict_args, expected_statuses', [
([], REQUESTED_STATUSES),
([('status', '')], REQUESTED_STATUSES),
([('status', 'garbage')], REQUESTED_STATUSES),
([('status', 'sending')], SENDING_STATUSES),
([('status', 'delivered')], DELIVERED_STATUSES),
([('status', 'failed')], FAILURE_STATUSES),
@pytest.mark.parametrize('notification_status, expected_status', [
('created', 'Sending'),
('sending', 'Sending'),
('delivered', 'Delivered'),
('failed', 'Failed'),
('temporary-failure', 'Phone not accepting messages right now'),
('permanent-failure', 'Phone number doesnt exist'),
('technical-failure', 'Technical failure'),
])
def test_status_filters(mocker, multidict_args, expected_statuses):
mocker.patch('app.main.views.notifications.current_app')
args = MultiDict(multidict_args)
args['status'] = get_status_arg(args)
assert sorted(args['status']) == sorted(expected_statuses)
@freeze_time("2016-01-01 11:09:00.061258")
def test_notification_status_page_shows_details(
client_request,
mock_get_notification,
service_one,
fake_uuid,
):
page = client_request.get(
'main.view_notification',
service_id=service_one['id'],
notification_id=fake_uuid
)
assert page.find('div', {'class': 'sms-message-wrapper'}).text.strip() == 'service one: template content'
assert ' '.join(page.find('tbody').find('tr').text.split()) == '07123456789 Delivered 1 January at 11:10am'
mock_get_notification.assert_called_with(
service_one['id'],
fake_uuid
)
@pytest.mark.parametrize('notification_status, expected_big_number_vals', [
('created', [1, 1, 0, 0]),
('sending', [1, 1, 0, 0]),
('delivered', [1, 0, 1, 0]),
('temporary-failure', [1, 0, 0, 1]),
])
def test_notification_status_page_shows_correct_numbers(
client_request,
mocker,
service_one,
fake_uuid,
notification_status,
expected_big_number_vals
expected_status,
):
mock_get_notification(mocker, fake_uuid, notification_status=notification_status)
_mock_get_notification = mock_get_notification(
mocker,
fake_uuid,
notification_status=notification_status
)
page = client_request.get(
'main.view_notification',
@@ -74,5 +43,17 @@ def test_notification_status_page_shows_correct_numbers(
notification_id=fake_uuid
)
big_numbers = page.find_all('div', {'class': 'big-number-number'})
assert expected_big_number_vals == [int(num.text.strip()) for num in big_numbers]
assert normalize_spaces(page.select('.sms-message-recipient')[0].text) == (
'To: 07123456789'
)
assert normalize_spaces(page.select('.sms-message-wrapper')[0].text) == (
'service one: hello Jo'
)
assert normalize_spaces(page.select('.ajax-block-container p')[0].text) == (
expected_status
)
_mock_get_notification.assert_called_with(
service_one['id'],
fake_uuid
)

View File

@@ -1144,7 +1144,9 @@ def mock_get_notifications_with_no_notifications(mocker):
status=None,
limit_days=None,
include_jobs=None,
include_from_test_key=None):
include_from_test_key=None,
to=None,
):
return notification_json(service_id, rows=0)
return mocker.patch(
@@ -1651,7 +1653,8 @@ def mock_get_notification(mocker, fake_uuid, notification_status='delivered'):
'name': 'Test User',
'email_address': 'test@user.gov.uk'
}
noti['template'] = template_json(service_id, str(generate_uuid()))
noti['personalisation'] = {'name': 'Jo'}
noti['template'] = template_json(service_id, str(generate_uuid()), content='hello ((name))')
return noti
return mocker.patch(