From 9e2f3909bca29093847d050b96f9111caaf50f82 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Nov 18 2016 11:09:33 +0000 Subject: [PATCH 1/8] Adjust the API documentation, PR status is a string not a boolean Fixes https://pagure.io/pagure/issue/1586 --- diff --git a/pagure/api/fork.py b/pagure/api/fork.py index dc6ceb9..0693e6d 100644 --- a/pagure/api/fork.py +++ b/pagure/api/fork.py @@ -109,7 +109,7 @@ def api_pull_request_views(repo, username=None, namespace=None): "name": "pingou" } }, - "status": true, + "status": "Open", "title": "test pull-request", "uid": "1431414800", "updated_on": "1431414800", @@ -237,7 +237,7 @@ def api_pull_request_view(repo, requestid, username=None, namespace=None): "name": "pingou" } }, - "status": true, + "status": "Open", "title": "test pull-request", "uid": "1431414800", "updated_on": "1431414800", From e33592ba238773b8bcfbfe40686d690e37831ce2 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Nov 18 2016 11:09:33 +0000 Subject: [PATCH 2/8] Include the arguments provided when listing all the projects in the API Fixes https://pagure.io/pagure/issue/1583 --- diff --git a/pagure/api/project.py b/pagure/api/project.py index cab5fb3..60cbf7b 100644 --- a/pagure/api/project.py +++ b/pagure/api/project.py @@ -167,7 +167,13 @@ def api_projects(): jsonout = flask.jsonify({ 'total_projects': len(projects), - 'projects': [p.to_json(api=True, public=True) for p in projects] + 'projects': [p.to_json(api=True, public=True) for p in projects], + 'args': { + 'tags': tags, + 'username': username, + 'fork': fork, + 'pattern': pattern, + } }) return jsonout diff --git a/tests/test_pagure_flask_api_project.py b/tests/test_pagure_flask_api_project.py index be97a62..59d0cdf 100644 --- a/tests/test_pagure_flask_api_project.py +++ b/tests/test_pagure_flask_api_project.py @@ -146,6 +146,14 @@ class PagureFlaskApiProjecttests(tests.Modeltests): self.assertDictEqual( data, { + "args": { + "fork": None, + "pattern": None, + "tags": [ + "infra" + ], + "username": None + }, "total_projects": 1, "projects": [ { @@ -182,6 +190,12 @@ class PagureFlaskApiProjecttests(tests.Modeltests): self.assertDictEqual( data, { + "args": { + "fork": None, + "pattern": None, + "tags": [], + "username": "pingou", + }, "total_projects": 3, "projects": [ { @@ -260,6 +274,14 @@ class PagureFlaskApiProjecttests(tests.Modeltests): self.assertDictEqual( data, { + "args": { + "fork": None, + "pattern": None, + "tags": [ + "infra" + ], + "username": "pingou" + }, "total_projects": 1, "projects": [ { From ec7d5edb495cab0c255029478cfefbdc68a78297 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Nov 18 2016 11:09:33 +0000 Subject: [PATCH 3/8] Include the JSON representation of the issue created in the API output Fixes https://pagure.io/pagure/issue/1582 --- diff --git a/pagure/api/issue.py b/pagure/api/issue.py index 9df2ff3..99cafd9 100644 --- a/pagure/api/issue.py +++ b/pagure/api/issue.py @@ -133,6 +133,7 @@ def api_new_issue(repo, username=None, namespace=None): SESSION.commit() output['message'] = 'Issue created' + output['issue'] = issue.to_json(public=True) except SQLAlchemyError as err: # pragma: no cover SESSION.rollback() APP.logger.exception(err) diff --git a/tests/test_pagure_flask_api_issue.py b/tests/test_pagure_flask_api_issue.py index 8be5d27..e855680 100644 --- a/tests/test_pagure_flask_api_issue.py +++ b/tests/test_pagure_flask_api_issue.py @@ -113,9 +113,34 @@ class PagureFlaskApiIssuetests(tests.Modeltests): '/api/0/test/new_issue', data=data, headers=headers) self.assertEqual(output.status_code, 200) data = json.loads(output.data) + data['issue']['date_created'] = '1479458613' self.assertDictEqual( data, - {'message': 'Issue created'} + { + "issue": { + "assignee": None, + "blocks": [], + "close_status": None, + "closed_at": None, + "comments": [], + "content": "This issue needs attention", + "custom_fields": [], + "date_created": "1479458613", + "depends": [], + "id": 1, + "milestone": None, + "priority": None, + "private": False, + "status": "Open", + "tags": [], + "title": "test issue", + "user": { + "fullname": "PY C", + "name": "pingou" + } + }, + "message": "Issue created" + } ) def test_api_view_issues(self): From 2d6f9d2c4b493d8e593332e383fbc5a7213d0b25 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Nov 18 2016 11:09:33 +0000 Subject: [PATCH 4/8] Fix the API documentation for creating new issues Fixes https://pagure.io/pagure/issue/1578 --- diff --git a/pagure/api/issue.py b/pagure/api/issue.py index 99cafd9..fd59be7 100644 --- a/pagure/api/issue.py +++ b/pagure/api/issue.py @@ -46,18 +46,18 @@ def api_new_issue(repo, username=None, namespace=None): Input ^^^^^ - +--------------+----------+--------------+-----------------------------+ - | Key | Type | Optionality | Description | - +==============+==========+==============+=============================+ - | ``title`` | string | Mandatory | The title of the issue | - +--------------+----------+--------------+-----------------------------+ - | ``content`` | string | Mandatory | | The description of the | - | | | | issue | - +--------------+----------+--------------+-----------------------------+ - | ``private`` | boolean | Optional | | Include this key if | - | | | | you want a private issue | - | | | | to be created | - +--------------+----------+--------------+-----------------------------+ + +-------------------+--------+-------------+---------------------------+ + | Key | Type | Optionality | Description | + +===================+========+=============+===========================+ + | ``title`` | string | Mandatory | The title of the issue | + +-------------------+--------+-------------+---------------------------+ + | ``issue_content`` | string | Mandatory | | The description of the | + | | | | issue | + +-------------------+--------+-------------+---------------------------+ + | ``private`` | boolean| Optional | | Include this key if | + | | | | you want a private issue| + | | | | to be created | + +-------------------+--------+-------------+---------------------------+ Sample response ^^^^^^^^^^^^^^^ From b86451bfb54aa2f72ccd37cd538e9f10fca5eba9 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Nov 18 2016 11:09:33 +0000 Subject: [PATCH 5/8] Adjust the API documentation for api_new_issue --- diff --git a/pagure/api/issue.py b/pagure/api/issue.py index fd59be7..0ef30bd 100644 --- a/pagure/api/issue.py +++ b/pagure/api/issue.py @@ -65,6 +65,28 @@ def api_new_issue(repo, username=None, namespace=None): :: { + "issue": { + "assignee": null, + "blocks": [], + "close_status": null, + "closed_at": null, + "comments": [], + "content": "This issue needs attention", + "custom_fields": [], + "date_created": "1479458613", + "depends": [], + "id": 1, + "milestone": null, + "priority": null, + "private": false, + "status": "Open", + "tags": [], + "title": "test issue", + "user": { + "fullname": "PY C", + "name": "pingou" + } + }, "message": "Issue created" } From 9a9e4281845b59b3633af9dca24afe15fee85632 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Nov 18 2016 11:09:33 +0000 Subject: [PATCH 6/8] Support: 'false', '', False, 'False', 0 and '0' as false values wtforms gives us a way to specify a list of False values to consider in boolean fields. With this selection we should be covered for most use case. Fixes https://pagure.io/pagure/issue/1579 --- diff --git a/pagure/forms.py b/pagure/forms.py index 21f5bf8..3663446 100644 --- a/pagure/forms.py +++ b/pagure/forms.py @@ -139,6 +139,7 @@ class ProjectForm(ProjectFormSimplified): create_readme = wtforms.BooleanField( 'Create README', [wtforms.validators.optional()], + false_values=('false', '', False, 'False', 0, '0'), ) namespace = wtforms.SelectField( 'Project Namespace', @@ -173,6 +174,7 @@ class IssueFormSimplied(PagureForm): private = wtforms.BooleanField( 'Private', [wtforms.validators.optional()], + false_values=('false', '', False, 'False', 0, '0'), ) @@ -326,6 +328,7 @@ class UpdateIssueForm(PagureForm): private = wtforms.BooleanField( 'Private', [wtforms.validators.optional()], + false_values=('false', '', False, 'False', 0, '0'), ) close_status = wtforms.SelectField( 'Closed as', @@ -621,5 +624,6 @@ class SubscribtionForm(PagureForm): status = wtforms.BooleanField( 'Subscription status', [wtforms.validators.optional()], + false_values=('false', '', False, 'False', 0, '0'), ) diff --git a/tests/test_pagure_flask_api_issue.py b/tests/test_pagure_flask_api_issue.py index e855680..5c01348 100644 --- a/tests/test_pagure_flask_api_issue.py +++ b/tests/test_pagure_flask_api_issue.py @@ -143,6 +143,250 @@ class PagureFlaskApiIssuetests(tests.Modeltests): } ) + # Valid request, with private='false' + data = { + 'title': 'test issue', + 'issue_content': 'This issue needs attention', + 'private': 'false', + } + + output = self.app.post( + '/api/0/test/new_issue', data=data, headers=headers) + self.assertEqual(output.status_code, 200) + data = json.loads(output.data) + data['issue']['date_created'] = '1479458613' + self.assertDictEqual( + data, + { + "issue": { + "assignee": None, + "blocks": [], + "close_status": None, + "closed_at": None, + "comments": [], + "content": "This issue needs attention", + "custom_fields": [], + "date_created": "1479458613", + "depends": [], + "id": 2, + "milestone": None, + "priority": None, + "private": False, + "status": "Open", + "tags": [], + "title": "test issue", + "user": { + "fullname": "PY C", + "name": "pingou" + } + }, + "message": "Issue created" + } + ) + + # Valid request, with private=False + data = { + 'title': 'test issue', + 'issue_content': 'This issue needs attention', + 'private': False + } + + output = self.app.post( + '/api/0/test/new_issue', data=data, headers=headers) + self.assertEqual(output.status_code, 200) + data = json.loads(output.data) + data['issue']['date_created'] = '1479458613' + self.assertDictEqual( + data, + { + "issue": { + "assignee": None, + "blocks": [], + "close_status": None, + "closed_at": None, + "comments": [], + "content": "This issue needs attention", + "custom_fields": [], + "date_created": "1479458613", + "depends": [], + "id": 3, + "milestone": None, + "priority": None, + "private": False, + "status": "Open", + "tags": [], + "title": "test issue", + "user": { + "fullname": "PY C", + "name": "pingou" + } + }, + "message": "Issue created" + } + ) + + # Valid request, with private='False' + data = { + 'title': 'test issue', + 'issue_content': 'This issue needs attention', + 'private': 'False' + } + + output = self.app.post( + '/api/0/test/new_issue', data=data, headers=headers) + self.assertEqual(output.status_code, 200) + data = json.loads(output.data) + data['issue']['date_created'] = '1479458613' + self.assertDictEqual( + data, + { + "issue": { + "assignee": None, + "blocks": [], + "close_status": None, + "closed_at": None, + "comments": [], + "content": "This issue needs attention", + "custom_fields": [], + "date_created": "1479458613", + "depends": [], + "id": 4, + "milestone": None, + "priority": None, + "private": False, + "status": "Open", + "tags": [], + "title": "test issue", + "user": { + "fullname": "PY C", + "name": "pingou" + } + }, + "message": "Issue created" + } + ) + + # Valid request, with private=0 + data = { + 'title': 'test issue', + 'issue_content': 'This issue needs attention', + 'private': 0 + } + + output = self.app.post( + '/api/0/test/new_issue', data=data, headers=headers) + self.assertEqual(output.status_code, 200) + data = json.loads(output.data) + data['issue']['date_created'] = '1479458613' + self.assertDictEqual( + data, + { + "issue": { + "assignee": None, + "blocks": [], + "close_status": None, + "closed_at": None, + "comments": [], + "content": "This issue needs attention", + "custom_fields": [], + "date_created": "1479458613", + "depends": [], + "id": 5, + "milestone": None, + "priority": None, + "private": False, + "status": "Open", + "tags": [], + "title": "test issue", + "user": { + "fullname": "PY C", + "name": "pingou" + } + }, + "message": "Issue created" + } + ) + + # Private issue: True + data = { + 'title': 'test issue', + 'issue_content': 'This issue needs attention', + 'private': True, + } + output = self.app.post( + '/api/0/test/new_issue', data=data, headers=headers) + self.assertEqual(output.status_code, 200) + data = json.loads(output.data) + data['issue']['date_created'] = '1479458613' + self.assertDictEqual( + data, + { + "issue": { + "assignee": None, + "blocks": [], + "close_status": None, + "closed_at": None, + "comments": [], + "content": "This issue needs attention", + "custom_fields": [], + "date_created": "1479458613", + "depends": [], + "id": 6, + "milestone": None, + "priority": None, + "private": True, + "status": "Open", + "tags": [], + "title": "test issue", + "user": { + "fullname": "PY C", + "name": "pingou" + } + }, + "message": "Issue created" + } + ) + + # Private issue: 1 + data = { + 'title': 'test issue', + 'issue_content': 'This issue needs attention', + 'private': 1, + } + output = self.app.post( + '/api/0/test/new_issue', data=data, headers=headers) + self.assertEqual(output.status_code, 200) + data = json.loads(output.data) + data['issue']['date_created'] = '1479458613' + self.assertDictEqual( + data, + { + "issue": { + "assignee": None, + "blocks": [], + "close_status": None, + "closed_at": None, + "comments": [], + "content": "This issue needs attention", + "custom_fields": [], + "date_created": "1479458613", + "depends": [], + "id": 7, + "milestone": None, + "priority": None, + "private": True, + "status": "Open", + "tags": [], + "title": "test issue", + "user": { + "fullname": "PY C", + "name": "pingou" + } + }, + "message": "Issue created" + } + ) + def test_api_view_issues(self): """ Test the api_view_issues method of the flask api. """ self.test_api_new_issue() @@ -159,11 +403,305 @@ class PagureFlaskApiIssuetests(tests.Modeltests): } ) - # List all opened issues - output = self.app.get('/api/0/test/issues') + # List all opened issues + output = self.app.get('/api/0/test/issues') + self.assertEqual(output.status_code, 200) + data = json.loads(output.data) + for idx in range(len(data['issues'])): + data['issues'][idx]['date_created'] = '1431414800' + self.assertDictEqual( + data, + { + "args": { + "assignee": None, + "author": None, + "status": None, + "tags": [] + }, + "issues": [ + { + "assignee": None, + "blocks": [], + "close_status": None, + "closed_at": None, + "comments": [], + "content": "This issue needs attention", + "custom_fields": [], + "date_created": "1431414800", + "depends": [], + "id": 5, + "milestone": None, + "priority": None, + "private": False, + "status": "Open", + "tags": [], + "title": "test issue", + "user": { + "fullname": "PY C", + "name": "pingou" + } + }, + { + "assignee": None, + "blocks": [], + "close_status": None, + "closed_at": None, + "comments": [], + "content": "This issue needs attention", + "custom_fields": [], + "date_created": "1431414800", + "depends": [], + "id": 4, + "milestone": None, + "priority": None, + "private": False, + "status": "Open", + "tags": [], + "title": "test issue", + "user": { + "fullname": "PY C", + "name": "pingou" + } + }, + { + "assignee": None, + "blocks": [], + "close_status": None, + "closed_at": None, + "comments": [], + "content": "This issue needs attention", + "custom_fields": [], + "date_created": "1431414800", + "depends": [], + "id": 3, + "milestone": None, + "priority": None, + "private": False, + "status": "Open", + "tags": [], + "title": "test issue", + "user": { + "fullname": "PY C", + "name": "pingou" + } + }, + { + "assignee": None, + "blocks": [], + "close_status": None, + "closed_at": None, + "comments": [], + "content": "This issue needs attention", + "custom_fields": [], + "date_created": "1431414800", + "depends": [], + "id": 2, + "milestone": None, + "priority": None, + "private": False, + "status": "Open", + "tags": [], + "title": "test issue", + "user": { + "fullname": "PY C", + "name": "pingou" + } + }, + { + "assignee": None, + "blocks": [], + "close_status": None, + "closed_at": None, + "comments": [], + "content": "This issue needs attention", + "custom_fields": [], + "date_created": "1431414800", + "depends": [], + "id": 1, + "milestone": None, + "priority": None, + "private": False, + "status": "Open", + "tags": [], + "title": "test issue", + "user": { + "fullname": "PY C", + "name": "pingou" + } + } + ], + "total_issues": 5 + } + ) + + # Create private issue + repo = pagure.lib.get_project(self.session, 'test') + msg = pagure.lib.new_issue( + session=self.session, + repo=repo, + title='Test issue', + content='We should work on this', + user='pingou', + ticketfolder=None, + private=True, + ) + self.session.commit() + self.assertEqual(msg.title, 'Test issue') + + # Access issues un-authenticated + output = self.app.get('/api/0/test/issues') + self.assertEqual(output.status_code, 200) + data = json.loads(output.data) + for idx in range(len(data['issues'])): + data['issues'][idx]['date_created'] = '1431414800' + self.assertDictEqual( + data, + { + "args": { + "assignee": None, + "author": None, + "status": None, + "tags": [] + }, + "issues": [ + { + "assignee": None, + "blocks": [], + "close_status": None, + "closed_at": None, + "comments": [], + "content": "This issue needs attention", + "custom_fields": [], + "date_created": "1431414800", + "depends": [], + "id": 5, + "milestone": None, + "priority": None, + "private": False, + "status": "Open", + "tags": [], + "title": "test issue", + "user": { + "fullname": "PY C", + "name": "pingou" + } + }, + { + "assignee": None, + "blocks": [], + "close_status": None, + "closed_at": None, + "comments": [], + "content": "This issue needs attention", + "custom_fields": [], + "date_created": "1431414800", + "depends": [], + "id": 4, + "milestone": None, + "priority": None, + "private": False, + "status": "Open", + "tags": [], + "title": "test issue", + "user": { + "fullname": "PY C", + "name": "pingou" + } + }, + { + "assignee": None, + "blocks": [], + "close_status": None, + "closed_at": None, + "comments": [], + "content": "This issue needs attention", + "custom_fields": [], + "date_created": "1431414800", + "depends": [], + "id": 3, + "milestone": None, + "priority": None, + "private": False, + "status": "Open", + "tags": [], + "title": "test issue", + "user": { + "fullname": "PY C", + "name": "pingou" + } + }, + { + "assignee": None, + "blocks": [], + "close_status": None, + "closed_at": None, + "comments": [], + "content": "This issue needs attention", + "custom_fields": [], + "date_created": "1431414800", + "depends": [], + "id": 2, + "milestone": None, + "priority": None, + "private": False, + "status": "Open", + "tags": [], + "title": "test issue", + "user": { + "fullname": "PY C", + "name": "pingou" + } + }, + { + "assignee": None, + "blocks": [], + "close_status": None, + "closed_at": None, + "comments": [], + "content": "This issue needs attention", + "custom_fields": [], + "date_created": "1431414800", + "depends": [], + "id": 1, + "milestone": None, + "priority": None, + "private": False, + "status": "Open", + "tags": [], + "title": "test issue", + "user": { + "fullname": "PY C", + "name": "pingou" + } + } + ], + "total_issues": 5 + } + ) + headers = {'Authorization': 'token aaabbbccc'} + + # Access issues authenticated but non-existing token + output = self.app.get('/api/0/test/issues', headers=headers) + self.assertEqual(output.status_code, 401) + + # Create a new token for another user + item = pagure.lib.model.Token( + id='bar_token', + user_id=2, + project_id=1, + expiration=datetime.datetime.utcnow() + datetime.timedelta( + days=30) + ) + self.session.add(item) + + headers = {'Authorization': 'token bar_token'} + + # Access issues authenticated but wrong token + output = self.app.get('/api/0/test/issues', headers=headers) self.assertEqual(output.status_code, 200) data = json.loads(output.data) - data['issues'][0]['date_created'] = '1431414800' + for idx in range(len(data['issues'])): + data['issues'][idx]['date_created'] = '1431414800' self.assertDictEqual( data, { @@ -173,17 +711,104 @@ class PagureFlaskApiIssuetests(tests.Modeltests): "status": None, "tags": [] }, - "total_issues": 1, "issues": [ { "assignee": None, "blocks": [], + "close_status": None, + "closed_at": None, + "comments": [], + "content": "This issue needs attention", + "custom_fields": [], + "date_created": "1431414800", + "depends": [], + "id": 5, + "milestone": None, + "priority": None, + "private": False, + "status": "Open", + "tags": [], + "title": "test issue", + "user": { + "fullname": "PY C", + "name": "pingou" + } + }, + { + "assignee": None, + "blocks": [], + "close_status": None, + "closed_at": None, + "comments": [], + "content": "This issue needs attention", + "custom_fields": [], + "date_created": "1431414800", + "depends": [], + "id": 4, + "milestone": None, + "priority": None, + "private": False, + "status": "Open", + "tags": [], + "title": "test issue", + "user": { + "fullname": "PY C", + "name": "pingou" + } + }, + { + "assignee": None, + "blocks": [], + "close_status": None, + "closed_at": None, + "comments": [], + "content": "This issue needs attention", + "custom_fields": [], + "date_created": "1431414800", + "depends": [], + "id": 3, + "milestone": None, + "priority": None, + "private": False, + "status": "Open", + "tags": [], + "title": "test issue", + "user": { + "fullname": "PY C", + "name": "pingou" + } + }, + { + "assignee": None, + "blocks": [], + "close_status": None, + "closed_at": None, "comments": [], "content": "This issue needs attention", "custom_fields": [], "date_created": "1431414800", + "depends": [], + "id": 2, + "milestone": None, + "priority": None, + "private": False, + "status": "Open", + "tags": [], + "title": "test issue", + "user": { + "fullname": "PY C", + "name": "pingou" + } + }, + { + "assignee": None, + "blocks": [], "close_status": None, "closed_at": None, + "comments": [], + "content": "This issue needs attention", + "custom_fields": [], + "date_created": "1431414800", "depends": [], "id": 1, "milestone": None, @@ -197,29 +822,19 @@ class PagureFlaskApiIssuetests(tests.Modeltests): "name": "pingou" } } - ] + ], + "total_issues": 5 } ) - # Create private issue - repo = pagure.lib.get_project(self.session, 'test') - msg = pagure.lib.new_issue( - session=self.session, - repo=repo, - title='Test issue', - content='We should work on this', - user='pingou', - ticketfolder=None, - private=True, - ) - self.session.commit() - self.assertEqual(msg.title, 'Test issue') + headers = {'Authorization': 'token aaabbbcccddd'} - # Access issues un-authenticated - output = self.app.get('/api/0/test/issues') + # Access issues authenticated correctly + output = self.app.get('/api/0/test/issues', headers=headers) self.assertEqual(output.status_code, 200) data = json.loads(output.data) - data['issues'][0]['date_created'] = '1431414800' + for idx in range(len(data['issues'])): + data['issues'][idx]['date_created'] = '1431414800' self.assertDictEqual( data, { @@ -229,19 +844,106 @@ class PagureFlaskApiIssuetests(tests.Modeltests): "status": None, "tags": [] }, - "total_issues": 1, "issues": [ { "assignee": None, "blocks": [], + "close_status": None, + "closed_at": None, + "comments": [], + "content": "We should work on this", + "custom_fields": [], + "date_created": "1431414800", + "depends": [], + "id": 8, + "milestone": None, + "priority": None, + "private": True, + "status": "Open", + "tags": [], + "title": "Test issue", + "user": { + "fullname": "PY C", + "name": "pingou" + } + }, + { + "assignee": None, + "blocks": [], + "close_status": None, + "closed_at": None, + "comments": [], + "content": "This issue needs attention", + "custom_fields": [], + "date_created": "1431414800", + "depends": [], + "id": 7, + "milestone": None, + "priority": None, + "private": True, + "status": "Open", + "tags": [], + "title": "test issue", + "user": { + "fullname": "PY C", + "name": "pingou" + } + }, + { + "assignee": None, + "blocks": [], + "close_status": None, + "closed_at": None, + "comments": [], + "content": "This issue needs attention", + "custom_fields": [], + "date_created": "1431414800", + "depends": [], + "id": 6, + "milestone": None, + "priority": None, + "private": True, + "status": "Open", + "tags": [], + "title": "test issue", + "user": { + "fullname": "PY C", + "name": "pingou" + } + }, + { + "assignee": None, + "blocks": [], + "close_status": None, + "closed_at": None, + "comments": [], + "content": "This issue needs attention", + "custom_fields": [], + "date_created": "1431414800", + "depends": [], + "id": 5, + "milestone": None, + "priority": None, + "private": False, + "status": "Open", + "tags": [], + "title": "test issue", + "user": { + "fullname": "PY C", + "name": "pingou" + } + }, + { + "assignee": None, + "blocks": [], + "close_status": None, + "closed_at": None, "comments": [], "content": "This issue needs attention", "custom_fields": [], "date_created": "1431414800", - "close_status": None, - "closed_at": None, "depends": [], - "id": 1, + "id": 4, "milestone": None, "priority": None, "private": False, @@ -252,55 +954,18 @@ class PagureFlaskApiIssuetests(tests.Modeltests): "fullname": "PY C", "name": "pingou" } - } - ] - } - ) - headers = {'Authorization': 'token aaabbbccc'} - - # Access issues authenticated but non-existing token - output = self.app.get('/api/0/test/issues', headers=headers) - self.assertEqual(output.status_code, 401) - - # Create a new token for another user - item = pagure.lib.model.Token( - id='bar_token', - user_id=2, - project_id=1, - expiration=datetime.datetime.utcnow() + datetime.timedelta( - days=30) - ) - self.session.add(item) - - headers = {'Authorization': 'token bar_token'} - - # Access issues authenticated but wrong token - output = self.app.get('/api/0/test/issues', headers=headers) - self.assertEqual(output.status_code, 200) - data = json.loads(output.data) - data['issues'][0]['date_created'] = '1431414800' - self.assertDictEqual( - data, - { - "args": { - "assignee": None, - "author": None, - "status": None, - "tags": [] - }, - "total_issues": 1, - "issues": [ + }, { "assignee": None, "blocks": [], + "close_status": None, + "closed_at": None, "comments": [], "content": "This issue needs attention", "custom_fields": [], "date_created": "1431414800", - "close_status": None, - "closed_at": None, "depends": [], - "id": 1, + "id": 3, "milestone": None, "priority": None, "private": False, @@ -311,47 +976,24 @@ class PagureFlaskApiIssuetests(tests.Modeltests): "fullname": "PY C", "name": "pingou" } - } - ] - } - ) - - headers = {'Authorization': 'token aaabbbcccddd'} - - # Access issues authenticated correctly - output = self.app.get('/api/0/test/issues', headers=headers) - self.assertEqual(output.status_code, 200) - data = json.loads(output.data) - data['issues'][0]['date_created'] = '1431414800' - data['issues'][1]['date_created'] = '1431414800' - self.assertDictEqual( - data, - { - "args": { - "assignee": None, - "author": None, - "status": None, - "tags": [] - }, - "total_issues": 2, - "issues": [ + }, { "assignee": None, "blocks": [], + "close_status": None, + "closed_at": None, "comments": [], - "content": "We should work on this", + "content": "This issue needs attention", "custom_fields": [], "date_created": "1431414800", - "close_status": None, - "closed_at": None, "depends": [], "id": 2, "milestone": None, "priority": None, - "private": True, + "private": False, "status": "Open", "tags": [], - "title": "Test issue", + "title": "test issue", "user": { "fullname": "PY C", "name": "pingou" @@ -360,12 +1002,12 @@ class PagureFlaskApiIssuetests(tests.Modeltests): { "assignee": None, "blocks": [], + "close_status": None, + "closed_at": None, "comments": [], "content": "This issue needs attention", "custom_fields": [], "date_created": "1431414800", - "close_status": None, - "closed_at": None, "depends": [], "id": 1, "milestone": None, @@ -379,7 +1021,8 @@ class PagureFlaskApiIssuetests(tests.Modeltests): "name": "pingou" } } - ] + ], + "total_issues": 8 } ) @@ -423,8 +1066,8 @@ class PagureFlaskApiIssuetests(tests.Modeltests): output = self.app.get('/api/0/test/issues?status=All', headers=headers) self.assertEqual(output.status_code, 200) data = json.loads(output.data) - data['issues'][0]['date_created'] = '1431414800' - data['issues'][1]['date_created'] = '1431414800' + for idx in range(len(data['issues'])): + data['issues'][idx]['date_created'] = '1431414800' self.assertDictEqual( data, { @@ -434,53 +1077,186 @@ class PagureFlaskApiIssuetests(tests.Modeltests): "status": "All", "tags": [] }, - "total_issues": 2, "issues": [ - { - "assignee": None, - "blocks": [], - "comments": [], - "content": "We should work on this", - "custom_fields": [], - "date_created": "1431414800", - "close_status": None, - "closed_at": None, - "depends": [], - "id": 2, - "milestone": None, - "priority": None, - "private": True, - "status": "Open", - "tags": [], - "title": "Test issue", - "user": { - "fullname": "PY C", - "name": "pingou" - } - }, - { - "assignee": None, - "blocks": [], - "comments": [], - "content": "This issue needs attention", - "custom_fields": [], - "date_created": "1431414800", - "close_status": None, - "closed_at": None, - "depends": [], - "id": 1, - "milestone": None, - "priority": None, - "private": False, - "status": "Open", - "tags": [], - "title": "test issue", - "user": { - "fullname": "PY C", - "name": "pingou" - } - } - ], + { + "assignee": None, + "blocks": [], + "close_status": None, + "closed_at": None, + "comments": [], + "content": "We should work on this", + "custom_fields": [], + "date_created": "1431414800", + "depends": [], + "id": 8, + "milestone": None, + "priority": None, + "private": True, + "status": "Open", + "tags": [], + "title": "Test issue", + "user": { + "fullname": "PY C", + "name": "pingou" + } + }, + { + "assignee": None, + "blocks": [], + "close_status": None, + "closed_at": None, + "comments": [], + "content": "This issue needs attention", + "custom_fields": [], + "date_created": "1431414800", + "depends": [], + "id": 7, + "milestone": None, + "priority": None, + "private": True, + "status": "Open", + "tags": [], + "title": "test issue", + "user": { + "fullname": "PY C", + "name": "pingou" + } + }, + { + "assignee": None, + "blocks": [], + "close_status": None, + "closed_at": None, + "comments": [], + "content": "This issue needs attention", + "custom_fields": [], + "date_created": "1431414800", + "depends": [], + "id": 6, + "milestone": None, + "priority": None, + "private": True, + "status": "Open", + "tags": [], + "title": "test issue", + "user": { + "fullname": "PY C", + "name": "pingou" + } + }, + { + "assignee": None, + "blocks": [], + "close_status": None, + "closed_at": None, + "comments": [], + "content": "This issue needs attention", + "custom_fields": [], + "date_created": "1431414800", + "depends": [], + "id": 5, + "milestone": None, + "priority": None, + "private": False, + "status": "Open", + "tags": [], + "title": "test issue", + "user": { + "fullname": "PY C", + "name": "pingou" + } + }, + { + "assignee": None, + "blocks": [], + "close_status": None, + "closed_at": None, + "comments": [], + "content": "This issue needs attention", + "custom_fields": [], + "date_created": "1431414800", + "depends": [], + "id": 4, + "milestone": None, + "priority": None, + "private": False, + "status": "Open", + "tags": [], + "title": "test issue", + "user": { + "fullname": "PY C", + "name": "pingou" + } + }, + { + "assignee": None, + "blocks": [], + "close_status": None, + "closed_at": None, + "comments": [], + "content": "This issue needs attention", + "custom_fields": [], + "date_created": "1431414800", + "depends": [], + "id": 3, + "milestone": None, + "priority": None, + "private": False, + "status": "Open", + "tags": [], + "title": "test issue", + "user": { + "fullname": "PY C", + "name": "pingou" + } + }, + { + "assignee": None, + "blocks": [], + "close_status": None, + "closed_at": None, + "comments": [], + "content": "This issue needs attention", + "custom_fields": [], + "date_created": "1431414800", + "depends": [], + "id": 2, + "milestone": None, + "priority": None, + "private": False, + "status": "Open", + "tags": [], + "title": "test issue", + "user": { + "fullname": "PY C", + "name": "pingou" + } + }, + { + "assignee": None, + "blocks": [], + "close_status": None, + "closed_at": None, + "comments": [], + "content": "This issue needs attention", + "custom_fields": [], + "date_created": "1431414800", + "depends": [], + "id": 1, + "milestone": None, + "priority": None, + "private": False, + "status": "Open", + "tags": [], + "title": "test issue", + "user": { + "fullname": "PY C", + "name": "pingou" + } + } + ], + "total_issues": 8 + } ) @@ -559,7 +1335,7 @@ class PagureFlaskApiIssuetests(tests.Modeltests): self.assertEqual(msg.title, 'Test issue') # Access private issue un-authenticated - output = self.app.get('/api/0/test/issue/2') + output = self.app.get('/api/0/test/issue/6') self.assertEqual(output.status_code, 403) data = json.loads(output.data) self.assertDictEqual( @@ -573,7 +1349,7 @@ class PagureFlaskApiIssuetests(tests.Modeltests): headers = {'Authorization': 'token aaabbbccc'} # Access private issue authenticated but non-existing token - output = self.app.get('/api/0/test/issue/2', headers=headers) + output = self.app.get('/api/0/test/issue/6', headers=headers) self.assertEqual(output.status_code, 401) data = json.loads(output.data) self.assertEqual(pagure.api.APIERROR.EINVALIDTOK.name, @@ -593,7 +1369,7 @@ class PagureFlaskApiIssuetests(tests.Modeltests): headers = {'Authorization': 'token bar_token'} # Access private issue authenticated but wrong token - output = self.app.get('/api/0/test/issue/2', headers=headers) + output = self.app.get('/api/0/test/issue/6', headers=headers) self.assertEqual(output.status_code, 403) data = json.loads(output.data) self.assertDictEqual( @@ -607,7 +1383,7 @@ class PagureFlaskApiIssuetests(tests.Modeltests): headers = {'Authorization': 'token aaabbbcccddd'} # Access private issue authenticated correctly - output = self.app.get('/api/0/test/issue/2', headers=headers) + output = self.app.get('/api/0/test/issue/6', headers=headers) self.assertEqual(output.status_code, 200) data = json.loads(output.data) data['date_created'] = '1431414800' @@ -617,19 +1393,19 @@ class PagureFlaskApiIssuetests(tests.Modeltests): "assignee": None, "blocks": [], "comments": [], - "content": "We should work on this", + "content": "This issue needs attention", "custom_fields": [], "date_created": "1431414800", "close_status": None, "closed_at": None, "depends": [], - "id": 2, + "id": 6, "milestone": None, "priority": None, "private": True, "status": "Open", "tags": [], - "title": "Test issue", + "title": "test issue", "user": { "fullname": "PY C", "name": "pingou" @@ -654,7 +1430,7 @@ class PagureFlaskApiIssuetests(tests.Modeltests): "close_status": None, "closed_at": None, "depends": [], - "id": 2, + "id": 8, "milestone": None, "priority": None, "private": True, From f7fcaa639e23fe3e0f31145734ba8d2acc90643c Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Nov 18 2016 11:09:33 +0000 Subject: [PATCH 7/8] 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."]} } ) From fe29d7e31e469b31dd8a335e71346a24d690a286 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Nov 18 2016 11:25:00 +0000 Subject: [PATCH 8/8] Fix running the unit-tests - In the blame_loc filter we were starting at 0 instead of starting at 1 that broke the tests - There was one more place where we're reporting errors of invalid input --- diff --git a/pagure/ui/filters.py b/pagure/ui/filters.py index afb865d..b4f3dee 100644 --- a/pagure/ui/filters.py +++ b/pagure/ui/filters.py @@ -284,7 +284,7 @@ def blame_loc(loc, repo, username, blame): '' '' - % ({'cnt': idx}) + % ({'cnt': idx + 1}) ) output.append( diff --git a/tests/test_pagure_flask_api_auth.py b/tests/test_pagure_flask_api_auth.py index ac9e574..64bed25 100644 --- a/tests/test_pagure_flask_api_auth.py +++ b/tests/test_pagure_flask_api_auth.py @@ -129,6 +129,10 @@ class PagureFlaskApiAuthtests(tests.Modeltests): { "error": "Invalid or incomplete input submited", "error_code": "EINVALIDREQ", + "errors": { + "issue_content": ["This field is required."], + "title": ["This field is required."] + } } )