diff --git a/app/__init__.py b/app/__init__.py index 8db5ca6d1..67e53be0a 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -6,10 +6,12 @@ from flask import (Flask, session, Markup, escape, render_template, make_respons from flask._compat import string_types from flask_login import LoginManager from flask_wtf import CsrfProtect -from werkzeug.exceptions import abort +from notifications_python_client import HTTPError from pygments import highlight from pygments.lexers import JavascriptLexer from pygments.formatters import HtmlFormatter +from werkzeug.exceptions import abort + from app.notify_client.api_client import NotificationsAdminAPIClient from app.notify_client.api_key_api_client import ApiKeyApiClient from app.notify_client.user_api_client import UserApiClient @@ -176,10 +178,33 @@ def useful_headers_after_request(response): def register_errorhandlers(application): - def render_error(error): - # If a HTTPException, pull the `code` attribute; default to 500 - error_code = getattr(error, 'code', 500) + def _error_response(error_code): resp = make_response(render_template("error/{0}.html".format(error_code)), error_code) return useful_headers_after_request(resp) - for errcode in [401, 404, 403, 500]: - application.errorhandler(errcode)(render_error) + + @application.errorhandler(HTTPError) + def render_http_error(error): + error_code = getattr(error, 'code', 500) + if error_code not in [401, 404, 403, 500]: + error_code = 500 + return _error_response(error_code) + + @application.errorhandler(400) + def handle_bad_request(error): + return _error_response(404) + + @application.errorhandler(404) + def handle_not_found(error): + return _error_response(404) + + @application.errorhandler(403) + def handle_not_authorized(error): + return _error_response(403) + + @application.errorhandler(401) + def handle_no_permissions(error): + return _error_response(401) + + @application.errorhandler(Exception) + def handle_bad_request(error): + return _error_response(500) diff --git a/app/main/dao/services_dao.py b/app/main/dao/services_dao.py index 4104b0ff4..1e867801b 100644 --- a/app/main/dao/services_dao.py +++ b/app/main/dao/services_dao.py @@ -1,7 +1,6 @@ -from flask import url_for, abort, current_app +from flask import url_for, current_app from app import notifications_api_client from app.utils import BrowsableItem -from notifications_python_client.errors import HTTPError def insert_new_service(service_name, user_id): @@ -26,15 +25,7 @@ def get_service_by_id(id_): def get_service_by_id_or_404(id_): - try: - return notifications_api_client.get_service(id_)['data'] - except KeyError: - abort(404) - except HTTPError as e: - if e.status_code == 404: - abort(404) - else: - raise e + return notifications_api_client.get_service(id_)['data'] def get_services(user_id=None): diff --git a/app/main/dao/templates_dao.py b/app/main/dao/templates_dao.py index 7d07b7d0e..c6a22ccf2 100644 --- a/app/main/dao/templates_dao.py +++ b/app/main/dao/templates_dao.py @@ -1,7 +1,6 @@ -from flask import url_for, abort +from flask import url_for from app import notifications_api_client from app.utils import BrowsableItem -from notifications_python_client.errors import HTTPError def insert_service_template(name, type_, content, service_id, subject=None): @@ -19,13 +18,7 @@ def get_service_templates(service_id): def get_service_template_or_404(service_id, template_id): - try: - return notifications_api_client.get_service_template(service_id, template_id) - except HTTPError as e: - if e.status_code == 404: - abort(404) - else: - raise e + return notifications_api_client.get_service_template(service_id, template_id) def delete_service_template(service_id, template_id): diff --git a/app/main/dao/users_dao.py b/app/main/dao/users_dao.py index 932a98cd3..9b27d5839 100644 --- a/app/main/dao/users_dao.py +++ b/app/main/dao/users_dao.py @@ -1,11 +1,6 @@ -from datetime import datetime from notifications_python_client import HTTPError -from sqlalchemy.orm import load_only - from app import login_manager -from app.main.encryption import hashpw - from app import user_api_client # diff --git a/app/main/views/dashboard.py b/app/main/views/dashboard.py index e0c2a84ab..e43f66b15 100644 --- a/app/main/views/dashboard.py +++ b/app/main/views/dashboard.py @@ -9,36 +9,25 @@ from flask_login import login_required from app.main import main from app.main.dao.services_dao import get_service_by_id from app.main.dao import templates_dao -from notifications_python_client.errors import HTTPError from app import job_api_client @main.route("/services//dashboard") @login_required def service_dashboard(service_id): - try: - templates = templates_dao.get_service_templates(service_id)['data'] - jobs = job_api_client.get_job(service_id)['data'] - except HTTPError as e: - if e.status_code == 404: - abort(404) - else: - raise e - try: - service = get_service_by_id(service_id) - session['service_name'] = service['data']['name'] - session['service_id'] = service['data']['id'] + templates = templates_dao.get_service_templates(service_id)['data'] + jobs = job_api_client.get_job(service_id)['data'] + + service = get_service_by_id(service_id) + session['service_name'] = service['data']['name'] + session['service_id'] = service['data']['id'] + + if session.get('invited_user'): + session.pop('invited_user', None) + service_name = service['data']['name'] + message = 'You have sucessfully accepted your invitation and been added to {}'.format(service_name) + flash(message, 'default_with_tick') - if session.get('invited_user'): - session.pop('invited_user', None) - service_name = service['data']['name'] - message = 'You have sucessfully accepted your invitation and been added to {}'.format(service_name) - flash(message, 'default_with_tick') - except HTTPError as e: - if e.status_code == 404: - abort(404) - else: - raise e return render_template( 'views/service_dashboard.html', jobs=list(reversed(jobs))[:5], diff --git a/app/main/views/forgot_password.py b/app/main/views/forgot_password.py index 16818bf06..f63d8227f 100644 --- a/app/main/views/forgot_password.py +++ b/app/main/views/forgot_password.py @@ -1,7 +1,6 @@ from flask import ( render_template, ) -from notifications_python_client.errors import HTTPError from app.main import main from app.main.forms import ForgotPasswordForm @@ -12,11 +11,8 @@ from app import user_api_client def forgot_password(): form = ForgotPasswordForm() if form.validate_on_submit(): - try: - user_api_client.send_reset_password_url(form.email_address.data) - except HTTPError as e: - if e.status_code != 404: - raise e + user_api_client.send_reset_password_url(form.email_address.data) + return render_template('views/password-reset-sent.html') return render_template('views/forgot-password.html', form=form) diff --git a/app/main/views/invites.py b/app/main/views/invites.py index 72c63771d..0bc7ad3c5 100644 --- a/app/main/views/invites.py +++ b/app/main/views/invites.py @@ -2,12 +2,9 @@ from flask import ( redirect, url_for, session, - abort, - render_template, - flash -) + flash, + render_template) -from notifications_python_client.errors import HTTPError from app.main import main from app.main.dao.services_dao import get_service_by_id_or_404 @@ -19,38 +16,29 @@ from app import ( @main.route("/invitation/") def accept_invite(token): + invited_user = invite_api_client.check_token(token) - try: - invited_user = invite_api_client.check_token(token) + if invited_user.status == 'cancelled': + from_user = user_api_client.get_user(invited_user.from_user) + service = get_service_by_id_or_404(invited_user.service) + return render_template('views/cancelled-invitation.html', + from_user=from_user.name, + service_name=service['name']) - if invited_user.status == 'cancelled': - from_user = user_api_client.get_user(invited_user.from_user) - service = get_service_by_id_or_404(invited_user.service) - return render_template('views/cancelled-invitation.html', - from_user=from_user.name, - service_name=service['name']) + if invited_user.status == 'accepted': + session.pop('invited_user', None) + flash('You have already accepted this invitation', 'default') + return redirect(url_for('main.service_dashboard', service_id=invited_user.service)) - if invited_user.status == 'accepted': - session.pop('invited_user', None) - flash('You have already accepted this invitation', 'default') - return redirect(url_for('main.service_dashboard', service_id=invited_user.service)) + existing_user = user_api_client.get_user_by_email(invited_user.email_address) + session['invited_user'] = invited_user.serialize() - existing_user = user_api_client.get_user_by_email(invited_user.email_address) - session['invited_user'] = invited_user.serialize() + if existing_user: - if existing_user: - - user_api_client.add_user_to_service(invited_user.service, - existing_user.id, - invited_user.permissions) - invite_api_client.accept_invite(invited_user.service, invited_user.id) - return redirect(url_for('main.service_dashboard', service_id=invited_user.service)) - else: - return redirect(url_for('main.register_from_invite')) - - except HTTPError as e: - - if e.status_code == 404: - abort(404) - else: - raise e + user_api_client.add_user_to_service(invited_user.service, + existing_user.id, + invited_user.permissions) + invite_api_client.accept_invite(invited_user.service, invited_user.id) + return redirect(url_for('main.service_dashboard', service_id=invited_user.service)) + else: + return redirect(url_for('main.register_from_invite')) diff --git a/app/main/views/jobs.py b/app/main/views/jobs.py index 24949756e..9847ef2cc 100644 --- a/app/main/views/jobs.py +++ b/app/main/views/jobs.py @@ -8,7 +8,6 @@ from flask import ( jsonify ) from flask_login import login_required -from notifications_python_client.errors import HTTPError from utils.template import Template from app import job_api_client, notification_api_client @@ -20,89 +19,71 @@ from app.main.dao import services_dao @main.route("/services//jobs") @login_required def view_jobs(service_id): - try: - jobs = job_api_client.get_job(service_id)['data'] - return render_template( - 'views/jobs/jobs.html', - jobs=jobs, - service_id=service_id - ) - except HTTPError as e: - if e.status_code == 404: - abort(404) - else: - raise e + jobs = job_api_client.get_job(service_id)['data'] + return render_template( + 'views/jobs/jobs.html', + jobs=jobs, + service_id=service_id + ) @main.route("/services//jobs/") @login_required def view_job(service_id, job_id): service = services_dao.get_service_by_id_or_404(service_id) - try: - job = job_api_client.get_job(service_id, job_id)['data'] - template = templates_dao.get_service_template_or_404(service_id, job['template'])['data'] - notifications = notification_api_client.get_notifications_for_service(service_id, job_id) - finished = job['status'] == 'finished' - return render_template( - 'views/jobs/job.html', - notifications=notifications['notifications'], - counts={ - 'queued': 0 if finished else job['notification_count'], - 'sent': job['notification_count'] if finished else 0, - 'failed': 0, - 'cost': u'£0.00' - }, - uploaded_at=job['created_at'], - finished_at=job['updated_at'] if finished else None, - uploaded_file_name=job['original_file_name'], - template=Template( - template, - prefix=service['name'] if template['template_type'] == 'sms' else '' - ), - service_id=service_id, - service=service, - job_id=job_id - ) - except HTTPError as e: - if e.status_code == 404: - abort(404) - else: - raise e + job = job_api_client.get_job(service_id, job_id)['data'] + template = templates_dao.get_service_template_or_404(service_id, job['template'])['data'] + notifications = notification_api_client.get_notifications_for_service(service_id, job_id) + finished = job['status'] == 'finished' + return render_template( + 'views/jobs/job.html', + notifications=notifications['notifications'], + counts={ + 'queued': 0 if finished else job['notification_count'], + 'sent': job['notification_count'] if finished else 0, + 'failed': 0, + 'cost': u'£0.00' + }, + uploaded_at=job['created_at'], + finished_at=job['updated_at'] if finished else None, + uploaded_file_name=job['original_file_name'], + template=Template( + template, + prefix=service['name'] if template['template_type'] == 'sms' else '' + ), + service_id=service_id, + service=service, + job_id=job_id + ) @main.route("/services//jobs/.json") @login_required def view_job_updates(service_id, job_id): service = services_dao.get_service_by_id_or_404(service_id) - try: - job = job_api_client.get_job(service_id, job_id)['data'] - notifications = notification_api_client.get_notifications_for_service(service_id, job_id) - finished = job['status'] == 'finished' - return jsonify(**{ - 'counts': render_template( - 'partials/jobs/count.html', - counts={ - 'queued': 0 if finished else job['notification_count'], - 'sent': job['notification_count'] if finished else 0, - 'failed': 0, - 'cost': u'£0.00' - } - ), - 'notifications': render_template( - 'partials/jobs/notifications.html', - notifications=notifications['notifications'] - ), - 'status': render_template( - 'partials/jobs/status.html', - uploaded_at=job['created_at'], - finished_at=job['updated_at'] if finished else None - ), - }) - except HTTPError as e: - if e.status_code == 404: - abort(404) - else: - raise e + job = job_api_client.get_job(service_id, job_id)['data'] + notifications = notification_api_client.get_notifications_for_service(service_id, job_id) + finished = job['status'] == 'finished' + return jsonify(**{ + 'counts': render_template( + 'partials/jobs/count.html', + counts={ + 'queued': 0 if finished else job['notification_count'], + 'sent': job['notification_count'] if finished else 0, + 'failed': 0, + 'cost': u'£0.00' + } + ), + 'notifications': render_template( + 'partials/jobs/notifications.html', + notifications=notifications['notifications'] + ), + 'status': render_template( + 'partials/jobs/status.html', + uploaded_at=job['created_at'], + finished_at=job['updated_at'] if finished else None + ), + }) @main.route("/services//jobs//notification/") diff --git a/app/main/views/manage_users.py b/app/main/views/manage_users.py index 6848794a5..f40b81637 100644 --- a/app/main/views/manage_users.py +++ b/app/main/views/manage_users.py @@ -11,9 +11,6 @@ from flask_login import ( current_user ) -from notifications_python_client.errors import HTTPError -from app import user_api_client - from app.main import main from app.main.forms import ( InviteUserForm, diff --git a/app/main/views/register.py b/app/main/views/register.py index 476032784..f9d0fa47c 100644 --- a/app/main/views/register.py +++ b/app/main/views/register.py @@ -11,8 +11,6 @@ from flask import ( from flask.ext.login import current_user -from notifications_python_client.errors import HTTPError - from app.main import main from app.main.dao import users_dao from app.main.forms import ( @@ -65,17 +63,10 @@ def register_from_invite(): def _do_registration(form, service=None): if users_dao.is_email_unique(form.email_address.data): - try: - user = user_api_client.register_user(form.name.data, - form.email_address.data, - form.mobile_number.data, - form.password.data) - - except HTTPError as e: - if e.status_code == 404: - abort(404) - else: - raise e + user = user_api_client.register_user(form.name.data, + form.email_address.data, + form.mobile_number.data, + form.password.data) # TODO possibly there should be some exception handling # for sending sms and email codes. diff --git a/app/main/views/send.py b/app/main/views/send.py index b7578afa5..0254663c1 100644 --- a/app/main/views/send.py +++ b/app/main/views/send.py @@ -15,7 +15,6 @@ from flask import ( ) from flask_login import login_required, current_user -from notifications_python_client.errors import HTTPError from utils.template import Template from utils.recipients import RecipientCSV, first_column_heading @@ -80,13 +79,8 @@ def choose_template(service_id, template_type): if template_type not in ['email', 'sms']: abort(404) - try: - jobs = job_api_client.get_job(service_id)['data'] - except HTTPError as e: - if e.status_code == 404: - abort(404) - else: - raise e + jobs = job_api_client.get_job(service_id)['data'] + return render_template( 'views/choose-template.html', templates=[ diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py index 89718a9f9..6c7232e77 100644 --- a/app/main/views/service_settings.py +++ b/app/main/views/service_settings.py @@ -12,8 +12,7 @@ from flask_login import ( login_required, current_user ) - -from notifications_python_client.errors import HTTPError +from notifications_python_client import HTTPError from app.main.dao.services_dao import ( get_service_by_id, @@ -32,13 +31,8 @@ from app.main.forms import ConfirmPasswordForm, ServiceNameForm @login_required @user_has_permissions('manage_settings') def service_settings(service_id): - try: - service = get_service_by_id(service_id)['data'] - except HTTPError as e: - if e.status_code == 404: - abort(404) - else: - raise e + service = get_service_by_id(service_id)['data'] + return render_template( 'views/service-settings.html', service=service, @@ -50,13 +44,7 @@ def service_settings(service_id): @login_required @user_has_permissions('manage_settings') def service_name_change(service_id): - try: - service = get_service_by_id(service_id)['data'] - except HTTPError as e: - if e.status_code == 404: - abort(404) - else: - raise e + service = get_service_by_id(service_id)['data'] form = ServiceNameForm(find_all_service_names) @@ -75,13 +63,7 @@ def service_name_change(service_id): @login_required @user_has_permissions('manage_settings') def service_name_change_confirm(service_id): - try: - service = get_service_by_id(service_id)['data'] - except HTTPError as e: - if e.status_code == 404: - abort(404) - else: - raise e + service = get_service_by_id(service_id)['data'] # Validate password for form def _check_password(pwd): @@ -115,13 +97,7 @@ def service_name_change_confirm(service_id): @login_required @user_has_permissions('manage_settings') def service_request_to_go_live(service_id): - try: - service = get_service_by_id(service_id)['data'] - except HTTPError as e: - if e.status_code == 404: - abort(404) - else: - raise e + service = get_service_by_id(service_id)['data'] if request.method == 'GET': return render_template( 'views/service-settings/request-to-go-live.html', @@ -138,13 +114,7 @@ def service_request_to_go_live(service_id): @login_required @user_has_permissions('manage_settings') def service_status_change(service_id): - try: - service = get_service_by_id(service_id)['data'] - except HTTPError as e: - if e.status_code == 404: - abort(404) - else: - raise e + service = get_service_by_id(service_id)['data'] if request.method == 'GET': return render_template( @@ -160,13 +130,7 @@ def service_status_change(service_id): @login_required @user_has_permissions('manage_settings') def service_status_change_confirm(service_id): - try: - service = get_service_by_id(service_id)['data'] - except HTTPError as e: - if e.status_code == 404: - abort(404) - else: - raise e + service = get_service_by_id(service_id)['data'] # Validate password for form def _check_password(pwd): @@ -189,13 +153,7 @@ def service_status_change_confirm(service_id): @login_required @user_has_permissions('manage_settings') def service_delete(service_id): - try: - service = get_service_by_id(service_id)['data'] - except HTTPError as e: - if e.status_code == 404: - abort(404) - else: - raise e + service = get_service_by_id(service_id)['data'] if request.method == 'GET': return render_template( @@ -211,13 +169,7 @@ def service_delete(service_id): @login_required @user_has_permissions('manage_settings') def service_delete_confirm(service_id): - try: - service = get_service_by_id(service_id)['data'] - except HTTPError as e: - if e.status_code == 404: - abort(404) - else: - raise e + service = get_service_by_id(service_id)['data'] # Validate password for form def _check_password(pwd): @@ -225,13 +177,7 @@ def service_delete_confirm(service_id): form = ConfirmPasswordForm(_check_password) if form.validate_on_submit(): - try: - service = delete_service(service_id) - except HTTPError as e: - if e.status_code == 404: - abort(404) - else: - raise e + service = delete_service(service_id) return redirect(url_for('.choose_service')) return render_template( diff --git a/app/main/views/verify.py b/app/main/views/verify.py index c4549762b..a85b8607a 100644 --- a/app/main/views/verify.py +++ b/app/main/views/verify.py @@ -2,12 +2,9 @@ from flask import ( render_template, redirect, session, - url_for, - abort + url_for ) -from notifications_python_client.errors import HTTPError - from flask_login import login_user from app.main import main @@ -32,11 +29,6 @@ def verify(): activated_user = users_dao.activate_user(user) login_user(activated_user) return redirect(url_for('main.add_service', first='first')) - except HTTPError as e: - if e.status_code == 404: - abort(404) - else: - raise e finally: session.pop('user_details', None) diff --git a/app/notify_client/user_api_client.py b/app/notify_client/user_api_client.py index ba15b90ba..4ffc8456c 100644 --- a/app/notify_client/user_api_client.py +++ b/app/notify_client/user_api_client.py @@ -1,5 +1,3 @@ -import json - from notifications_python_client.notifications import BaseAPIClient from notifications_python_client.errors import HTTPError diff --git a/tests/app/main/views/test_register.py b/tests/app/main/views/test_register.py index d61513784..9849da99f 100644 --- a/tests/app/main/views/test_register.py +++ b/tests/app/main/views/test_register.py @@ -44,7 +44,7 @@ def test_process_register_creates_new_user(app_, assert mock_register_user.called -def test_process_register_returns_400_when_mobile_number_is_invalid(app_, +def test_process_register_returns_200_when_mobile_number_is_invalid(app_, mock_send_verify_code, mock_get_user_by_email_not_found, mock_login):