Refactored register_errorhandlers so that it handles HTTPError

Remove most cases where we catch HTTPError

app/__init__.py

@@ -6,6 +6,7 @@ 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 notifications_python_client import HTTPError
 from werkzeug.exceptions import abort
 from pygments import highlight
 from pygments.lexers import JavascriptLexer
@@ -176,10 +177,10 @@ def useful_headers_after_request(response):
 
 
 def register_errorhandlers(application):
-    def render_error(error):
+    @application.errorhandler(HTTPError)
-        # If a HTTPException, pull the `code` attribute; default to 500
+    def render_http_error(error):
         error_code = getattr(error, 'code', 500)
+        if error_code not in [401, 404, 403, 500]:
+            error_code = 500
         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)

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
 
 
 def get_services(user_id=None):

app/main/dao/templates_dao.py

@@ -22,10 +22,7 @@ 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(e.status_code)
-            abort(404)
-        else:
-            raise e
 
 
 def delete_service_template(service_id, template_id):

app/main/views/dashboard.py

@@ -16,15 +16,9 @@ from app import job_api_client
 @main.route("/services/<service_id>/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']
@@ -34,11 +28,7 @@ def service_dashboard(service_id):
         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],

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
         return render_template('views/password-reset-sent.html')
 
     return render_template('views/forgot-password.html', form=form)

app/main/views/invites.py

@@ -2,11 +2,9 @@ from flask import (
     redirect,
     url_for,
     session,
-    abort,
     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
@@ -18,9 +16,6 @@ from app import (
 
 @main.route("/invitation/<token>")
 def accept_invite(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)
@@ -41,9 +36,3 @@ def accept_invite(token):
         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

app/main/views/jobs.py

@@ -20,25 +20,18 @@ from app.main.dao import services_dao
 @main.route("/services/<service_id>/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
 
 
 @main.route("/services/<service_id>/jobs/<job_id>")
 @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)
@@ -63,18 +56,12 @@ def view_job(service_id, job_id):
         service=service,
         job_id=job_id
     )
-    except HTTPError as e:
-        if e.status_code == 404:
-            abort(404)
-        else:
-            raise e
 
 
 @main.route("/services/<service_id>/jobs/<job_id>.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'
@@ -98,11 +85,6 @@ def view_job_updates(service_id, job_id):
             finished_at=job['updated_at'] if finished else None
         ),
     })
-    except HTTPError as e:
-        if e.status_code == 404:
-            abort(404)
-        else:
-            raise e
 
 
 @main.route("/services/<service_id>/jobs/<job_id>/notification/<string:notification_id>")

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,

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 (
@@ -56,18 +54,11 @@ 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
-
         # TODO possibly there should be some exception handling
         # for sending sms and email codes.
         # How do we report to the user there is a problem with

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
     return render_template(
         'views/choose-template.html',
         templates=[

app/main/views/service_settings.py

@@ -13,8 +13,6 @@ from flask_login import (
     current_user
 )
 
-from notifications_python_client.errors import HTTPError
-
 from app.main.dao.services_dao import (
     get_service_by_id,
     delete_service,
@@ -31,13 +29,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
     return render_template(
         'views/service-settings.html',
         service=service,
@@ -49,13 +42,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
 
     form = ServiceNameForm()
 
@@ -74,13 +61,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
 
     # Validate password for form
     def _check_password(pwd):
@@ -104,13 +85,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
     if request.method == 'GET':
         return render_template(
             'views/service-settings/request-to-go-live.html',
@@ -127,13 +102,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
 
     if request.method == 'GET':
         return render_template(
@@ -149,13 +118,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
 
     # Validate password for form
     def _check_password(pwd):
@@ -178,13 +141,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
 
     if request.method == 'GET':
         return render_template(
@@ -200,13 +157,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
 
     # Validate password for form
     def _check_password(pwd):
@@ -214,13 +165,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
         return redirect(url_for('.choose_service'))
 
     return render_template(

app/main/views/verify.py

@@ -2,12 +2,9 @@ from flask import (
     render_template,
     redirect,
     session,
-    url_for,
+    url_for
-    abort
 )
 
-from notifications_python_client.errors import HTTPError
-
 from flask_login import login_user
 
 from app.main import main
@@ -31,11 +28,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:
             del session['user_details']
 

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
 

tests/app/main/views/test_register.py

@@ -43,7 +43,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):