From f7fcaa639e23fe3e0f31145734ba8d2acc90643c Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Nov 18 2016 11:09:33 +0000 Subject: Report the form's errors when the input provided doesn't validate it This way when an user sends an invalid or incomplete requests, we will inform them about the fields that are missing or did not pass the validators. Fixes https://pagure.io/pagure/issue/1581 --- diff --git a/pagure/api/__init__.py b/pagure/api/__init__.py index 66727d5..37a29f7 100644 --- a/pagure/api/__init__.py +++ b/pagure/api/__init__.py @@ -179,19 +179,19 @@ def api_method(function): APP.logger.exception(err) if err.error_code in [APIERROR.ENOCODE]: - response = flask.jsonify( - { + output = { 'error': err.error, 'error_code': err.error_code.name } - ) else: - response = flask.jsonify( - { + output = { 'error': err.error_code.value, 'error_code': err.error_code.name, } - ) + + if err.errors: + output['errors'] = err.errors + response = flask.jsonify(output) response.status_code = err.status_code else: response = result diff --git a/pagure/api/fork.py b/pagure/api/fork.py index 0693e6d..9bc5083 100644 --- a/pagure/api/fork.py +++ b/pagure/api/fork.py @@ -549,7 +549,8 @@ def api_pull_request_add_comment( raise pagure.exceptions.APIError(400, error_code=APIERROR.EDBERROR) else: - raise pagure.exceptions.APIError(400, error_code=APIERROR.EINVALIDREQ) + raise pagure.exceptions.APIError( + 400, error_code=APIERROR.EINVALIDREQ, errors=form.errors) jsonout = flask.jsonify(output) return jsonout @@ -691,7 +692,8 @@ def api_pull_request_add_flag(repo, requestid, username=None, namespace=None): raise pagure.exceptions.APIError(400, error_code=APIERROR.EDBERROR) else: - raise pagure.exceptions.APIError(400, error_code=APIERROR.EINVALIDREQ) + raise pagure.exceptions.APIError( + 400, error_code=APIERROR.EINVALIDREQ, errors=form.errors) jsonout = flask.jsonify(output) return jsonout diff --git a/pagure/api/issue.py b/pagure/api/issue.py index 0ef30bd..d684be5 100644 --- a/pagure/api/issue.py +++ b/pagure/api/issue.py @@ -162,7 +162,8 @@ def api_new_issue(repo, username=None, namespace=None): raise pagure.exceptions.APIError(400, error_code=APIERROR.EDBERROR) else: - raise pagure.exceptions.APIError(400, error_code=APIERROR.EINVALIDREQ) + raise pagure.exceptions.APIError( + 400, error_code=APIERROR.EINVALIDREQ, errors=form.errors) jsonout = flask.jsonify(output) return jsonout @@ -621,7 +622,8 @@ def api_change_status_issue(repo, issueid, username=None, namespace=None): raise pagure.exceptions.APIError(400, error_code=APIERROR.EDBERROR) else: - raise pagure.exceptions.APIError(400, error_code=APIERROR.EINVALIDREQ) + raise pagure.exceptions.APIError( + 400, error_code=APIERROR.EINVALIDREQ, errors=form.errors) jsonout = flask.jsonify(output) return jsonout @@ -719,7 +721,8 @@ def api_comment_issue(repo, issueid, username=None, namespace=None): raise pagure.exceptions.APIError(400, error_code=APIERROR.EDBERROR) else: - raise pagure.exceptions.APIError(400, error_code=APIERROR.EINVALIDREQ) + raise pagure.exceptions.APIError( + 400, error_code=APIERROR.EINVALIDREQ, errors=form.errors) jsonout = flask.jsonify(output) return jsonout @@ -817,7 +820,8 @@ def api_assign_issue(repo, issueid, username=None, namespace=None): raise pagure.exceptions.APIError(400, error_code=APIERROR.EDBERROR) else: - raise pagure.exceptions.APIError(400, error_code=APIERROR.EINVALIDREQ) + raise pagure.exceptions.APIError( + 400, error_code=APIERROR.EINVALIDREQ, errors=form.errors) jsonout = flask.jsonify(output) return jsonout @@ -917,7 +921,8 @@ def api_subscribe_issue(repo, issueid, username=None, namespace=None): raise pagure.exceptions.APIError(400, error_code=APIERROR.EDBERROR) else: - raise pagure.exceptions.APIError(400, error_code=APIERROR.EINVALIDREQ) + raise pagure.exceptions.APIError( + 400, error_code=APIERROR.EINVALIDREQ, errors=form.errors) jsonout = flask.jsonify(output) return jsonout diff --git a/pagure/api/project.py b/pagure/api/project.py index 60cbf7b..2d0bd6f 100644 --- a/pagure/api/project.py +++ b/pagure/api/project.py @@ -285,7 +285,8 @@ def api_new_project(): SESSION.rollback() raise pagure.exceptions.APIError(400, error_code=APIERROR.EDBERROR) else: - raise pagure.exceptions.APIError(400, error_code=APIERROR.EINVALIDREQ) + raise pagure.exceptions.APIError( + 400, error_code=APIERROR.EINVALIDREQ, errors=form.errors) jsonout = flask.jsonify(output) return jsonout @@ -370,7 +371,7 @@ def api_fork_project(): 400, error_code=APIERROR.EDBERROR) else: raise pagure.exceptions.APIError( - 400, error_code=APIERROR.EINVALIDREQ) + 400, error_code=APIERROR.EINVALIDREQ, errors=form.errors) jsonout = flask.jsonify(output) return jsonout diff --git a/pagure/exceptions.py b/pagure/exceptions.py index 8eeefc3..80ee351 100644 --- a/pagure/exceptions.py +++ b/pagure/exceptions.py @@ -33,10 +33,11 @@ class FileNotFoundException(PagureException): class APIError(PagureException): ''' Exception raised by the API when something goes wrong. ''' - def __init__(self, status_code, error_code, error=None): + def __init__(self, status_code, error_code, error=None, errors=None): self.status_code = status_code self.error_code = error_code self.error = error + self.errors = errors class BranchNotFoundException(PagureException): diff --git a/tests/test_pagure_flask_api_fork.py b/tests/test_pagure_flask_api_fork.py index 291a986..acb31eb 100644 --- a/tests/test_pagure_flask_api_fork.py +++ b/tests/test_pagure_flask_api_fork.py @@ -636,6 +636,7 @@ class PagureFlaskApiForktests(tests.Modeltests): { "error": "Invalid or incomplete input submited", "error_code": "EINVALIDREQ", + "errors": {"comment": ["This field is required."]} } ) @@ -748,6 +749,7 @@ class PagureFlaskApiForktests(tests.Modeltests): { "error": "Invalid or incomplete input submited", "error_code": "EINVALIDREQ", + "errors": {"comment": ["This field is required."]} } ) diff --git a/tests/test_pagure_flask_api_issue.py b/tests/test_pagure_flask_api_issue.py index 5c01348..d07bc11 100644 --- a/tests/test_pagure_flask_api_issue.py +++ b/tests/test_pagure_flask_api_issue.py @@ -70,6 +70,10 @@ class PagureFlaskApiIssuetests(tests.Modeltests): { "error": "Invalid or incomplete input submited", "error_code": "EINVALIDREQ", + "errors": { + "issue_content": ["This field is required."], + "title": ["This field is required."] + } } ) @@ -100,6 +104,10 @@ class PagureFlaskApiIssuetests(tests.Modeltests): { "error": "Invalid or incomplete input submited", "error_code": "EINVALIDREQ", + "errors": { + "issue_content": ["This field is required."], + "title": ["This field is required."] + } } ) @@ -1561,6 +1569,7 @@ class PagureFlaskApiIssuetests(tests.Modeltests): { "error": "Invalid or incomplete input submited", "error_code": "EINVALIDREQ", + "errors": {"status": ["Not a valid choice"]} } ) @@ -1692,6 +1701,7 @@ class PagureFlaskApiIssuetests(tests.Modeltests): { "error": "Invalid or incomplete input submited", "error_code": "EINVALIDREQ", + "errors": {"comment": ["This field is required."]} } ) @@ -2008,6 +2018,7 @@ class PagureFlaskApiIssuetests(tests.Modeltests): { "error": "Invalid or incomplete input submited", "error_code": "EINVALIDREQ", + "errors": {"assignee": ["This field is required."]} } ) diff --git a/tests/test_pagure_flask_api_project.py b/tests/test_pagure_flask_api_project.py index 59d0cdf..0b20353 100644 --- a/tests/test_pagure_flask_api_project.py +++ b/tests/test_pagure_flask_api_project.py @@ -341,6 +341,10 @@ class PagureFlaskApiProjecttests(tests.Modeltests): { "error": "Invalid or incomplete input submited", "error_code": "EINVALIDREQ", + "errors": { + "name": ["This field is required."], + "description": ["This field is required."] + } } ) @@ -358,6 +362,7 @@ class PagureFlaskApiProjecttests(tests.Modeltests): { "error": "Invalid or incomplete input submited", "error_code": "EINVALIDREQ", + "errors": {"description": ["This field is required."]} } ) @@ -427,6 +432,7 @@ class PagureFlaskApiProjecttests(tests.Modeltests): { "error": "Invalid or incomplete input submited", "error_code": "EINVALIDREQ", + "errors": {"repo": ["This field is required."]} } ) @@ -444,6 +450,7 @@ class PagureFlaskApiProjecttests(tests.Modeltests): { "error": "Invalid or incomplete input submited", "error_code": "EINVALIDREQ", + "errors": {"repo": ["This field is required."]} } )