diff --git a/app/__init__.py b/app/__init__.py index d0ce1d245..8db493b0e 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -2,7 +2,7 @@ import os import re import dateutil -from flask import (Flask, session, Markup, escape, render_template, make_response) +from flask import (Flask, session, Markup, escape, render_template, make_response, current_app) from flask._compat import string_types from flask_login import LoginManager from flask_wtf import CsrfProtect @@ -203,4 +203,7 @@ def register_errorhandlers(application): @application.errorhandler(Exception) def handle_bad_request(error): + # We want the Flask in browser stacktrace + if current_app.config.get('DEBUG', None): + raise error return _error_response(500) diff --git a/app/assets/stylesheets/components/_previous-next-navigation.scss b/app/assets/stylesheets/components/_previous-next-navigation.scss new file mode 100644 index 000000000..a1cf5e0ea --- /dev/null +++ b/app/assets/stylesheets/components/_previous-next-navigation.scss @@ -0,0 +1,129 @@ +/* +Taken from the GOV.UK component at +https://github.com/alphagov/static/blob/a9d462e71709d2ff6348bcce7e8c625af2b86114/app/assets/stylesheets/govuk-component/_previous-and-next-navigation.scss +and +https://github.com/alphagov/static/blob/da8aeeaa749093eab30286d7fc9f965533b66f47/app/assets/stylesheets/styleguide/_conditionals2.scss +*/ +@import "_colours.scss"; +@import "_typography.scss"; + +// Media query helpers. These make producing IE layouts +// super easy. + +// These are desktop and down media queries + +// There is also a local version of this in Smartanswers. + +$is-ie: false !default; + +@mixin media-down($size: false, $max-width: false, $min-width: false) { + @if $is-ie == false { + @if $size == mobile { + @media (max-width: 640px){ + @content; + } + } @else if $size == tablet { + @media (max-width: 800px){ + @content; + } + } + } +} + +.govuk-previous-and-next-navigation { + + @include media-down(mobile) { + margin: 2em 0 0 0; + } + + display: block; + + ul { + margin: 0; + padding: 0; + } + + li { + @include core-16($line-height: (20 / 16)); + float: left; + list-style: none; + text-align: right; + margin: 0; + padding: 0; + width: 49%; + + a { + + @include ie-lte(7) { + height: 4.5em; + } + + display: block; + color: $link-colour; + text-decoration: none; + + &:hover, + &:active { + background-color: $canvas-colour; + } + + .pagination-part-title { + @include core-27($line-height: (33.75 / 27)); + margin-bottom: 0.1em; + display: block; + } + + } + + &.next-page { + float: right; + text-align: right; + } + + &.next-page a:before { + background: transparent file-url("arrow-sprite.png") no-repeat -102px -11px; + margin: -4px -32px 0 0; + display: block; + float: right; + width: 30px; + height: 38px; + content: " "; + } + + &.previous-page a:before { + background: transparent file-url("arrow-sprite.png") no-repeat -20px -11px; + margin: -4px 0 0 -32px; + display: block; + float: left; + width: 30px; + height: 38px; + content: " "; + } + + &.previous-page { + float: left; + text-align: left; + } + + &.previous-page a { + padding: 0.75em 0 0.75em 3em; + } + + &.next-page a { + padding: 0.75em 3em 0.75em 0; + } + + @include media-down(mobile) { + &.previous-page, + &.next-page { + float: none; + width: 100%; + } + + &.next-page a { + text-align: right; + } + } + } + +} \ No newline at end of file diff --git a/app/assets/stylesheets/main.scss b/app/assets/stylesheets/main.scss index e9c36ad7a..74db12573 100644 --- a/app/assets/stylesheets/main.scss +++ b/app/assets/stylesheets/main.scss @@ -48,6 +48,7 @@ $path: '/static/images/'; @import 'components/email-message'; @import 'components/api-key'; @import 'components/yes-no'; +@import "components/_previous-next-navigation"; @import 'views/job'; @import 'views/edit-template'; diff --git a/app/main/views/jobs.py b/app/main/views/jobs.py index 9847ef2cc..48ef1875c 100644 --- a/app/main/views/jobs.py +++ b/app/main/views/jobs.py @@ -5,7 +5,11 @@ import time from flask import ( render_template, abort, - jsonify + jsonify, + flash, + redirect, + request, + url_for ) from flask_login import login_required from utils.template import Template @@ -14,6 +18,7 @@ from app import job_api_client, notification_api_client from app.main import main from app.main.dao import templates_dao from app.main.dao import services_dao +from app.utils import (get_page_from_request, generate_previous_next_dict) @main.route("/services//jobs") @@ -86,6 +91,34 @@ def view_job_updates(service_id, job_id): }) +@main.route('/services//notifications') +@login_required +def view_notifications(service_id): + # 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)) + notifications = notification_api_client.get_notifications_for_service(service_id=service_id, page=page) + prev_page = None + if notifications['links'].get('prev', None): + prev_page = generate_previous_next_dict( + 'main.view_notifications', + {'service_id': service_id}, page - 1, 'Previous page', 'page {}'.format(page - 1)) + next_page = None + if notifications['links'].get('next', None): + next_page = generate_previous_next_dict( + 'main.view_notifications', + {'service_id': service_id}, page + 1, 'Next page', 'page {}'.format(page + 1)) + return render_template( + 'views/notifications.html', + service_id=service_id, + notifications=notifications['notifications'], + page=page, + prev_page=prev_page, + next_page=next_page + ) + + @main.route("/services//jobs//notification/") @login_required def view_notification(service_id, job_id, notification_id): diff --git a/app/templates/components/previous-next-navigation.html b/app/templates/components/previous-next-navigation.html new file mode 100644 index 000000000..9704ea939 --- /dev/null +++ b/app/templates/components/previous-next-navigation.html @@ -0,0 +1,22 @@ +{% macro previous_next_navigation(previous_page, next_page) %} + +{% endmacro %} \ No newline at end of file diff --git a/app/templates/main_nav.html b/app/templates/main_nav.html index 96c1c372b..311775ac5 100644 --- a/app/templates/main_nav.html +++ b/app/templates/main_nav.html @@ -25,4 +25,8 @@
  • Developer documentation
  • {% endif %} + diff --git a/app/templates/views/notifications.html b/app/templates/views/notifications.html new file mode 100644 index 000000000..fcc7ce90d --- /dev/null +++ b/app/templates/views/notifications.html @@ -0,0 +1,42 @@ +{% extends "withnav_template.html" %} +{% from "components/table.html" import list_table, field, right_aligned_field_heading %} +{% from "components/previous-next-navigation.html" import previous_next_navigation %} + +{% block page_title %} + Notifications activity – GOV.UK Notify +{% endblock %} + +{% block maincolumn_content %} + +

    Notifications activity

    + + {% call(item) list_table( + notifications, + caption="Recent activity", + caption_visible=False, + empty_message='You haven’t sent any notifications yet', + field_headings=['Recipient', 'Template', 'Type', 'Job', 'Status', 'Time']) + %} + {% call field() %} + {{ item.to }} + {% endcall %} + {% call field() %} + {{ item.template.name }} + {% endcall %} + {% call field() %} + {{ item.template.template_type }} + {% endcall %} + {% call field() %} + {% if item.job %} + {{ item.job.original_file_name }} + {% endif %} + {% endcall %} + {% call field() %} + {{ item.status }} + {% endcall %} + {% call field() %} + {{ item.created_at | format_datetime}} + {% endcall %} + {% endcall %} + {{ previous_next_navigation(prev_page, next_page) }} +{% endblock %} \ No newline at end of file diff --git a/app/templates/views/service_dashboard.html b/app/templates/views/service_dashboard.html index f7a6dc7fa..d21a0d991 100644 --- a/app/templates/views/service_dashboard.html +++ b/app/templates/views/service_dashboard.html @@ -65,7 +65,7 @@ {% if more_jobs_to_show %} {% if current_user.has_permissions(['send_texts', 'send_emails', 'send_letters']) %} {% endif %} {% endif %} diff --git a/app/utils.py b/app/utils.py index c6688a5bc..8f4ae81be 100644 --- a/app/utils.py +++ b/app/utils.py @@ -1,7 +1,7 @@ import re from functools import wraps -from flask import (abort, session) +from flask import (abort, session, request, url_for) class BrowsableItem(object): @@ -82,3 +82,21 @@ def get_errors_for_csv(recipients, template_type): errors.append("fill in {} empty cells".format(number_of_rows_with_missing_data)) return errors + + +def get_page_from_request(): + if 'page' in request.args: + try: + return int(request.args['page']) + except ValueError: + return None + else: + return 1 + + +def generate_previous_next_dict(view, view_dict, page, title, label): + return { + 'url': url_for(view, **view_dict, page=page), + 'title': title, + 'label': label + } diff --git a/config.py b/config.py index bb520a3ae..019917aa4 100644 --- a/config.py +++ b/config.py @@ -63,6 +63,7 @@ class Development(Config): class Test(Development): + DEBUG = True WTF_CSRF_ENABLED = False diff --git a/tests/__init__.py b/tests/__init__.py index a316c4342..4e8d93674 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -32,7 +32,7 @@ def service_json(id_, name, users, limit=1000, active=False, restricted=True): } -def template_json(id_, name, type_, content, service_id): +def template_json(service_id, id_=1, name="sample template", type_="sms", content="template content"): return { 'id': id_, 'name': name, @@ -117,13 +117,40 @@ def job_json(): return data -def notification_json(): +def notification_json(service_id, + job=None, + template=None, + to='07123456789', + status='sent', + sent_at=None, + created_at=None, + with_links=False): import datetime + if job is None: + job = job_json() + if template is None: + template = template_json(service_id) + if sent_at is None: + sent_at = str(datetime.datetime.now().time()) + if created_at is None: + created_at = str(datetime.datetime.now().time()) + links = {} + if with_links: + links = { + 'prev': '/service/{}/notifications'.format(service_id), + 'next': '/service/{}/notifications'.format(service_id), + 'last': '/service/{}/notifications'.format(service_id) + } data = { 'notifications': [{ - 'sent_at': str(datetime.datetime.now().time()) + 'to': to, + 'template': {'id': template['id'], 'name': template['name']}, + 'job': {'id': job['id'], 'file_name': job['file_name']}, + 'sent_at': sent_at, + 'status': status, + 'created_at': created_at } for i in range(5)], - 'links': {} + 'links': links } return data diff --git a/tests/app/main/views/test_jobs.py b/tests/app/main/views/test_jobs.py index 2b8d4e794..dcea08bfd 100644 --- a/tests/app/main/views/test_jobs.py +++ b/tests/app/main/views/test_jobs.py @@ -80,3 +80,45 @@ def test_should_show_updates_for_one_job_as_json( assert 'Recipient' in content['notifications'] assert 'Status' in content['notifications'] assert 'Started' in content['status'] + + +def test_should_show_notifications_for_a_service(app_, + service_one, + api_user_active, + mock_login, + mock_get_user, + mock_get_user_by_email, + mock_get_service, + mock_get_notifications): + with app_.test_request_context(): + with app_.test_client() as client: + client.login(api_user_active) + 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 = mock_get_notifications(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 + + +def test_should_show_notifications_for_a_service_with_next_previous(app_, + service_one, + api_user_active, + mock_login, + mock_get_user, + mock_get_user_by_email, + mock_get_service, + mock_get_notifications_with_previous_next): + with app_.test_request_context(): + with app_.test_client() as client: + client.login(api_user_active) + 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 diff --git a/tests/conftest.py b/tests/conftest.py index 9d5c88f39..f9693fe2c 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -158,7 +158,7 @@ def mock_delete_service(mocker, mock_get_service): def mock_get_service_template(mocker): def _create(service_id, template_id): template = template_json( - template_id, "Two week reminder", "sms", "Your vehicle tax is about to expire", service_id) + service_id, template_id, "Two week reminder", "sms", "Your vehicle tax is about to expire") return {'data': template} return mocker.patch( @@ -169,7 +169,7 @@ def mock_get_service_template(mocker): def mock_get_service_email_template(mocker): def _create(service_id, template_id): template = template_json( - template_id, "Two week reminder", "email", "Your vehicle tax is about to expire", service_id) + service_id, template_id, "Two week reminder", "email", "Your vehicle tax is about to expire") return {'data': template} return mocker.patch( @@ -180,7 +180,7 @@ def mock_get_service_email_template(mocker): def mock_create_service_template(mocker): def _create(name, type_, content, service): template = template_json( - 101, name, type_, content, service) + service, 101, name, type_, content) return {'data': template} return mocker.patch( @@ -192,7 +192,7 @@ def mock_create_service_template(mocker): def mock_update_service_template(mocker): def _update(id_, name, type_, content, service): template = template_json( - id_, name, type_, content, service) + service, id_, name, type_, content) return {'data': template} return mocker.patch( @@ -205,17 +205,13 @@ def mock_get_service_templates(mocker): def _create(service_id): return {'data': [ template_json( - 1, "sms_template_one", "sms", "sms template one content", service_id - ), + service_id, 1, "sms_template_one", "sms", "sms template one content"), template_json( - 2, "sms_template_two", "sms", "sms template two content", service_id - ), + service_id, 2, "sms_template_two", "sms", "sms template two content"), template_json( - 3, "email_template_one", "email", "email template one content", service_id - ), + service_id, 3, "email_template_one", "email", "email template one content"), template_json( - 4, "email_template_two", "email", "email template two content", service_id - ) + service_id, 4, "email_template_two", "email", "email template two content") ]} return mocker.patch( @@ -227,8 +223,7 @@ def mock_get_service_templates(mocker): def mock_delete_service_template(mocker): def _delete(service_id, template_id): template = template_json( - template_id, "Template to delete", - "sms", "content to be deleted", service_id) + service_id, template_id, "Template to delete", "sms", "content to be deleted") return {'data': template} return mocker.patch( @@ -580,8 +575,18 @@ def mock_get_jobs(mocker): @pytest.fixture(scope='function') def mock_get_notifications(mocker): - def _get_notifications(service_id, job_id): - return notification_json() + def _get_notifications(service_id, job_id=None, page=1): + return notification_json(service_id) + return mocker.patch( + 'app.notification_api_client.get_notifications_for_service', + side_effect=_get_notifications + ) + + +@pytest.fixture(scope='function') +def mock_get_notifications_with_previous_next(mocker): + def _get_notifications(service_id, job_id=None, page=1): + return notification_json(service_id, with_links=True) return mocker.patch( 'app.notification_api_client.get_notifications_for_service', side_effect=_get_notifications