From 0aaa5e0a4663ad8ead207171eb2cf0cf45a86f42 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jun 08 2016 09:50:48 +0000 Subject: [PATCH 1/6] Add an API endpoint to assign issue to someone --- diff --git a/pagure/api/issue.py b/pagure/api/issue.py index 4430177..805f34c 100644 --- a/pagure/api/issue.py +++ b/pagure/api/issue.py @@ -633,3 +633,91 @@ def api_comment_issue(repo, issueid, username=None): jsonout = flask.jsonify(output) return jsonout + + +@API.route('//issue//assign', methods=['POST']) +@API.route('/fork///issue//assign', methods=['POST']) +@api_login_required(acls=['issue_assign']) +@api_method +def api_assign_issue(repo, issueid, username=None): + """ + Assign an issue + --------------- + Assign an issue to someone. + + :: + + POST /api/0//issue//assign + + :: + + POST /api/0/fork///issue//assign + + Input + ^^^^^ + + +--------------+----------+---------------+---------------------------+ + | Key | Type | Optionality | Description | + +==============+==========+===============+===========================+ + | ``assignee`` | string | Mandatory | | The username of the user| + | | | | to assign the issue to. | + +--------------+----------+---------------+---------------------------+ + + Sample response + ^^^^^^^^^^^^^^^ + + :: + + { + "message": "Issue assigned" + } + + """ + repo = pagure.lib.get_project(SESSION, repo, user=username) + output = {} + + if repo is None: + raise pagure.exceptions.APIError(404, error_code=APIERROR.ENOPROJECT) + + if not repo.settings.get('issue_tracker', True): + raise pagure.exceptions.APIError( + 404, error_code=APIERROR.ETRACKERDISABLED) + + if repo.fullname != flask.g.token.project.fullname: + raise pagure.exceptions.APIError(401, error_code=APIERROR.EINVALIDTOK) + + issue = pagure.lib.search_issues(SESSION, repo, issueid=issueid) + + if issue is None or issue.project != repo: + raise pagure.exceptions.APIError(404, error_code=APIERROR.ENOISSUE) + + if issue.private and not is_repo_admin(repo) \ + and (not api_authenticated() or + not issue.user.user == flask.g.fas_user.username): + raise pagure.exceptions.APIError( + 403, error_code=APIERROR.EISSUENOTALLOWED) + + form = pagure.forms.AssigneIssueForm(csrf_enabled=False) + if form.validate_on_submit(): + assignee = form.assignee.data + try: + # New comment + message = pagure.lib.add_issue_assignee( + SESSION, + issue=issue, + assignee=assignee, + user=flask.g.fas_user.username, + ticketfolder=APP.config['TICKETS_FOLDER'], + ) + SESSION.commit() + output['message'] = message + except SQLAlchemyError as err: # pragma: no cover + SESSION.rollback() + APP.logger.exception(err) + raise pagure.exceptions.APIError(400, error_code=APIERROR.EDBERROR) + + else: + raise pagure.exceptions.APIError(400, error_code=APIERROR.EINVALIDREQ) + + jsonout = flask.jsonify(output) + return jsonout diff --git a/pagure/forms.py b/pagure/forms.py index 2aae91c..63868bb 100644 --- a/pagure/forms.py +++ b/pagure/forms.py @@ -262,6 +262,13 @@ class AddUserForm(wtf.Form): ) +class AssigneIssueForm(wtf.Form): + ''' Form to asiggn an user to an issue. ''' + assignee = wtforms.TextField( + 'Assignee *', + [wtforms.validators.Required()] + ) + class AddGroupForm(wtf.Form): ''' Form to add a group to a project. ''' group = wtforms.TextField( From c02e60ea9c486513f486b52b8671cd9fd6b2003b Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jun 08 2016 09:51:26 +0000 Subject: [PATCH 2/6] Add the new ACL to assign issue and fix the tests for it --- diff --git a/pagure/default_config.py b/pagure/default_config.py index 218639d..530644c 100644 --- a/pagure/default_config.py +++ b/pagure/default_config.py @@ -193,11 +193,12 @@ BLACKLISTED_GROUPS = ['forks'] ACLS = { 'create_project': 'Create a new project', + 'issue_assign': 'Assign issue to someone', 'issue_create': 'Create a new ticket against this project', 'issue_change_status': 'Change the status of a ticket of this project', 'issue_comment': 'Comment on a ticket of this project', - 'pull_request_merge': 'Merge a pull-request of this project', 'pull_request_close': 'Close a pull-request of this project', 'pull_request_comment': 'Comment on a pull-request of this project', 'pull_request_flag': 'Flag a pull-request of this project', + 'pull_request_merge': 'Merge a pull-request of this project', } diff --git a/tests/__init__.py b/tests/__init__.py index 0746c1a..13e8132 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -263,7 +263,7 @@ def create_tokens(session, user_id=1): def create_tokens_acl(session, token_id='aaabbbcccddd'): """ Create some acls for the tokens. """ - for aclid in range(8): + for aclid in range(len(pagure.APP.config['ACLS'])): item = pagure.lib.model.TokenAcl( token_id=token_id, acl_id=aclid + 1, diff --git a/tests/test_pagure_flask_api_fork.py b/tests/test_pagure_flask_api_fork.py index 9c689ea..a8e69c6 100644 --- a/tests/test_pagure_flask_api_fork.py +++ b/tests/test_pagure_flask_api_fork.py @@ -366,9 +366,11 @@ class PagureFlaskApiForktests(tests.Modeltests): ) self.session.add(item) self.session.commit() + + # Allow the token to close PR item = pagure.lib.model.TokenAcl( token_id='foobar_token', - acl_id=5, + acl_id=6, ) self.session.add(item) self.session.commit() @@ -478,9 +480,11 @@ class PagureFlaskApiForktests(tests.Modeltests): ) self.session.add(item) self.session.commit() + + # Allow the token to merge PR item = pagure.lib.model.TokenAcl( token_id='foobar_token', - acl_id=8, + acl_id=9, ) self.session.add(item) self.session.commit() From 3d10a762c3290b9bf2eaa12a54927bb70faf0cc7 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jun 08 2016 09:52:24 +0000 Subject: [PATCH 3/6] Add unit-tests for the API endpoint to assign issue --- diff --git a/tests/test_pagure_flask_api_issue.py b/tests/test_pagure_flask_api_issue.py index aa59574..ff288ad 100644 --- a/tests/test_pagure_flask_api_issue.py +++ b/tests/test_pagure_flask_api_issue.py @@ -1113,6 +1113,221 @@ class PagureFlaskApiIssuetests(tests.Modeltests): } ) + @patch('pagure.lib.git.update_git') + @patch('pagure.lib.notify.send_email') + def test_api_assign_issue(self, p_send_email, p_ugt): + """ Test the api_assign_issue method of the flask api. """ + p_send_email.return_value = True + p_ugt.return_value = True + + tests.create_projects(self.session) + tests.create_tokens(self.session) + tests.create_tokens_acl(self.session) + + headers = {'Authorization': 'token aaabbbcccddd'} + + # Invalid project + output = self.app.post('/api/0/foo/issue/1/assign', headers=headers) + self.assertEqual(output.status_code, 404) + data = json.loads(output.data) + self.assertDictEqual( + data, + { + "error": "Project not found", + "error_code": "ENOPROJECT", + } + ) + + # Valid token, wrong project + output = self.app.post('/api/0/test2/issue/1/assign', headers=headers) + self.assertEqual(output.status_code, 401) + data = json.loads(output.data) + self.assertDictEqual( + data, + { + "error": "Invalid or expired token. Please visit " \ + "https://pagure.org/ to get or renew your API token.", + "error_code": "EINVALIDTOK", + } + ) + + # No input + output = self.app.post('/api/0/test/issue/1/assign', headers=headers) + self.assertEqual(output.status_code, 404) + data = json.loads(output.data) + self.assertDictEqual( + data, + { + "error": "Issue not found", + "error_code": "ENOISSUE", + } + ) + + # Create normal issue + repo = pagure.lib.get_project(self.session, 'test') + msg = pagure.lib.new_issue( + session=self.session, + repo=repo, + title='Test issue #1', + content='We should work on this', + user='pingou', + ticketfolder=None, + private=False, + issue_uid='aaabbbccc#1', + ) + self.session.commit() + self.assertEqual(msg.title, 'Test issue #1') + + # Check comments before + repo = pagure.lib.get_project(self.session, 'test') + issue = pagure.lib.search_issues(self.session, repo, issueid=1) + self.assertEqual(len(issue.comments), 0) + + data = { + 'title': 'test issue', + } + + # Incomplete request + output = self.app.post( + '/api/0/test/issue/1/assign', data=data, headers=headers) + self.assertEqual(output.status_code, 400) + data = json.loads(output.data) + self.assertDictEqual( + data, + { + "error": "Invalid or incomplete input submited", + "error_code": "EINVALIDREQ", + } + ) + + # No change + repo = pagure.lib.get_project(self.session, 'test') + issue = pagure.lib.search_issues(self.session, repo, issueid=1) + self.assertEqual(issue.status, 'Open') + + data = { + 'assignee': 'pingou', + } + + # Valid request + output = self.app.post( + '/api/0/test/issue/1/assign', data=data, headers=headers) + self.assertEqual(output.status_code, 200) + data = json.loads(output.data) + self.assertDictEqual( + data, + {'message': 'Issue assigned'} + ) + + # One comment added + repo = pagure.lib.get_project(self.session, 'test') + issue = pagure.lib.search_issues(self.session, repo, issueid=1) + self.assertEqual(issue.assignee.user, 'pingou') + + # Create another project + item = pagure.lib.model.Project( + user_id=2, # foo + name='foo', + description='test project #3', + hook_token='aaabbbdddeee', + ) + self.session.add(item) + self.session.commit() + + # Create a token for pingou for this project + item = pagure.lib.model.Token( + id='pingou_foo', + user_id=1, + project_id=3, + expiration=datetime.datetime.utcnow() + datetime.timedelta( + days=30) + ) + self.session.add(item) + self.session.commit() + + # Give `issue_change_status` to this token when `issue_comment` + # is required + print [ + (t.id, t.name) + for t in self.session.query(pagure.lib.model.ACL).all() + ] + item = pagure.lib.model.TokenAcl( + token_id='pingou_foo', + acl_id=3, + ) + self.session.add(item) + self.session.commit() + + repo = pagure.lib.get_project(self.session, 'foo') + # Create private issue + msg = pagure.lib.new_issue( + session=self.session, + repo=repo, + title='Test issue', + content='We should work on this', + user='foo', + ticketfolder=None, + private=True, + issue_uid='aaabbbccc#2', + ) + self.session.commit() + self.assertEqual(msg.title, 'Test issue') + + # Check before + repo = pagure.lib.get_project(self.session, 'foo') + issue = pagure.lib.search_issues(self.session, repo, issueid=1) + self.assertEqual(len(issue.comments), 0) + + data = { + 'assignee': 'pingou', + } + headers = {'Authorization': 'token pingou_foo'} + + # Valid request but un-authorized + output = self.app.post( + '/api/0/foo/issue/1/assign', data=data, headers=headers) + self.assertEqual(output.status_code, 401) + data = json.loads(output.data) + self.assertDictEqual( + data, + { + "error": "Invalid or expired token. Please " + "visit https://pagure.org/ to get or renew your API token.", + "error_code": "EINVALIDTOK" + } + ) + + # No comment added + repo = pagure.lib.get_project(self.session, 'foo') + issue = pagure.lib.search_issues(self.session, repo, issueid=1) + self.assertEqual(len(issue.comments), 0) + + # Create token for user foo + item = pagure.lib.model.Token( + id='foo_token2', + user_id=2, + project_id=3, + expiration=datetime.datetime.utcnow() + datetime.timedelta(days=30) + ) + self.session.add(item) + self.session.commit() + tests.create_tokens_acl(self.session, token_id='foo_token2') + + data = { + 'assignee': 'pingou', + } + headers = {'Authorization': 'token foo_token2'} + + # Valid request and authorized + output = self.app.post( + '/api/0/foo/issue/1/assign', data=data, headers=headers) + self.assertEqual(output.status_code, 200) + data = json.loads(output.data) + self.assertDictEqual( + data, + {'message': 'Issue assigned'} + ) + if __name__ == '__main__': SUITE = unittest.TestLoader().loadTestsFromTestCase( From 43b3f73adde5e46dec02b6fe20c9ecd56d60c812 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jun 08 2016 12:18:31 +0000 Subject: [PATCH 4/6] Remove debug code --- diff --git a/tests/test_pagure_flask_api_issue.py b/tests/test_pagure_flask_api_issue.py index ff288ad..0d8c9ee 100644 --- a/tests/test_pagure_flask_api_issue.py +++ b/tests/test_pagure_flask_api_issue.py @@ -1247,10 +1247,6 @@ class PagureFlaskApiIssuetests(tests.Modeltests): # Give `issue_change_status` to this token when `issue_comment` # is required - print [ - (t.id, t.name) - for t in self.session.query(pagure.lib.model.ACL).all() - ] item = pagure.lib.model.TokenAcl( token_id='pingou_foo', acl_id=3, From 6a719b546815d86548656af16e549fa4e69f353e Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jun 11 2016 11:53:59 +0000 Subject: [PATCH 5/6] Rename the form AssigneIssueForm to AssignIssueForm --- diff --git a/pagure/api/issue.py b/pagure/api/issue.py index 805f34c..68e1674 100644 --- a/pagure/api/issue.py +++ b/pagure/api/issue.py @@ -697,7 +697,7 @@ def api_assign_issue(repo, issueid, username=None): raise pagure.exceptions.APIError( 403, error_code=APIERROR.EISSUENOTALLOWED) - form = pagure.forms.AssigneIssueForm(csrf_enabled=False) + form = pagure.forms.AssignIssueForm(csrf_enabled=False) if form.validate_on_submit(): assignee = form.assignee.data try: diff --git a/pagure/forms.py b/pagure/forms.py index 63868bb..a7d19f1 100644 --- a/pagure/forms.py +++ b/pagure/forms.py @@ -262,8 +262,8 @@ class AddUserForm(wtf.Form): ) -class AssigneIssueForm(wtf.Form): - ''' Form to asiggn an user to an issue. ''' +class AssignIssueForm(wtf.Form): + ''' Form to assign an user to an issue. ''' assignee = wtforms.TextField( 'Assignee *', [wtforms.validators.Required()] From b8fd2ce4dbb7bc8704d1a1181621df546a9fb169 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jun 11 2016 12:02:35 +0000 Subject: [PATCH 6/6] Small pep8 fix --- diff --git a/pagure/forms.py b/pagure/forms.py index a7d19f1..4d8fb47 100644 --- a/pagure/forms.py +++ b/pagure/forms.py @@ -269,6 +269,7 @@ class AssignIssueForm(wtf.Form): [wtforms.validators.Required()] ) + class AddGroupForm(wtf.Form): ''' Form to add a group to a project. ''' group = wtforms.TextField(