diff --git a/pagure/api/__init__.py b/pagure/api/__init__.py index e3a94b9..7630b9c 100644 --- a/pagure/api/__init__.py +++ b/pagure/api/__init__.py @@ -566,6 +566,7 @@ def api(): fork.api_pull_request_add_comment ) api_pull_request_add_flag_doc = load_doc(fork.api_pull_request_add_flag) + api_pull_request_assign_doc = load_doc(fork.api_pull_request_assign) api_version_doc = load_doc(api_version) api_whoami_doc = load_doc(api_whoami) @@ -631,6 +632,7 @@ def api(): api_pull_request_close_doc, api_pull_request_add_comment_doc, api_pull_request_add_flag_doc, + api_pull_request_assign_doc, ], users=[ api_users_doc, diff --git a/pagure/api/fork.py b/pagure/api/fork.py index 66da001..4dea521 100644 --- a/pagure/api/fork.py +++ b/pagure/api/fork.py @@ -1439,3 +1439,98 @@ def api_pull_request_diffstats(repo, requestid, username=None, namespace=None): jsonout = flask.jsonify(output) return jsonout + + +@API.route("//pull-request//assign", methods=["POST"]) +@API.route( + "///pull-request//assign", methods=["POST"] +) +@API.route( + "/fork///pull-request//assign", + methods=["POST"], +) +@API.route( + "/fork////pull-request//assign", + methods=["POST"], +) +@api_login_required(acls=["pull_request_assign", "pull_request_update"]) +@api_method +def api_pull_request_assign(repo, requestid, username=None, namespace=None): + """ + Assign a pull-request + --------------------- + Assign a pull-request to someone. + + :: + + POST /api/0//pull-request//assign + POST /api/0///pull-request//assign + + :: + + POST /api/0/fork///pull-request//assign + POST /api/0/fork////pull-request//assign + + Input + ^^^^^ + + +--------------+----------+---------------+---------------------------+ + | Key | Type | Optionality | Description | + +==============+==========+===============+===========================+ + | ``assignee`` | string | Mandatory | | The username of the user| + | | | | to assign the PR to. | + +--------------+----------+---------------+---------------------------+ + + Sample response + ^^^^^^^^^^^^^^^ + + :: + + { + "message": "pull-request assigned" + } + + """ # noqa + output = {} + repo = _get_repo(repo, username, namespace) + + _check_pull_request(repo) + _check_token(repo) + + request = _get_request(repo, requestid) + _check_pull_request_access(request, assignee=True) + + form = pagure.forms.AssignIssueForm(csrf_enabled=False) + if form.validate_on_submit(): + assignee = form.assignee.data or None + # Create our metadata comment object + try: + # New comment + message = pagure.lib.query.add_pull_request_assignee( + flask.g.session, + request=request, + assignee=assignee, + user=flask.g.fas_user.username, + ) + flask.g.session.commit() + if message: + pagure.lib.query.add_metadata_update_notif( + session=flask.g.session, + obj=request, + messages=message, + user=flask.g.fas_user.username, + ) + output["message"] = message + else: + output["message"] = "Nothing to change" + except pagure.exceptions.PagureException as err: # pragma: no cover + raise pagure.exceptions.APIError( + 400, error_code=APIERROR.ENOCODE, error=str(err) + ) + except SQLAlchemyError as err: # pragma: no cover + flask.g.session.rollback() + _log.exception(err) + raise pagure.exceptions.APIError(400, error_code=APIERROR.EDBERROR) + + jsonout = flask.jsonify(output) + return jsonout diff --git a/pagure/default_config.py b/pagure/default_config.py index 2c777cd..b43d5ce 100644 --- a/pagure/default_config.py +++ b/pagure/default_config.py @@ -345,6 +345,10 @@ ACLS = { "pull_request_subscribe": ( "Subscribe the user with this token to a pull-request" ), + "pull_request_assign": "Assign someone to a pull-request", + "pull_request_update": ( + "Update a pull-request (title, description, assignee...)" + ), "update_watch_status": "Update the watch status on a project", "pull_request_rebase": "Rebase a pull-request", } diff --git a/pagure/lib/query.py b/pagure/lib/query.py index 078aeac..1327637 100644 --- a/pagure/lib/query.py +++ b/pagure/lib/query.py @@ -646,7 +646,7 @@ def add_pull_request_assignee(session, request, assignee, user): ), ) - return "Request reset" + return "Request assignee reset" elif assignee is None and request.assignee is None: return diff --git a/tests/test_pagure_flask_api_fork_assign.py b/tests/test_pagure_flask_api_fork_assign.py new file mode 100644 index 0000000..eeb80c4 --- /dev/null +++ b/tests/test_pagure_flask_api_fork_assign.py @@ -0,0 +1,265 @@ +# -*- coding: utf-8 -*- + +""" + (c) 2019 - Copyright Red Hat Inc + + Authors: + Pierre-Yves Chibon + +""" + +from __future__ import unicode_literals, absolute_import + +import arrow +import copy +import datetime +import unittest +import shutil +import sys +import time +import os + +import flask +import json +import munch +from mock import patch, MagicMock +from sqlalchemy.exc import SQLAlchemyError + +sys.path.insert(0, os.path.join(os.path.dirname( + os.path.abspath(__file__)), '..')) + +import pagure.lib.query +import tests + + +class PagureFlaskApiForkAssigntests(tests.SimplePagureTest): + """ Tests for the flask API of pagure for assigning a PR """ + + maxDiff = None + + @patch('pagure.lib.git.update_git', MagicMock(return_value=True)) + @patch('pagure.lib.notify.send_email', MagicMock(return_value=True)) + def setUp(self): + """ Set up the environnment, ran before every tests. """ + super(PagureFlaskApiForkAssigntests, self).setUp() + + tests.create_projects(self.session) + tests.add_content_git_repo( + os.path.join(self.path, "repos", "test.git")) + + # Fork + project = pagure.lib.query.get_authorized_project( + self.session, 'test') + task = pagure.lib.query.fork_project( + session=self.session, + user='pingou', + repo=project, + ) + self.session.commit() + self.assertEqual( + task.get(), + {'endpoint': 'ui_ns.view_repo', + 'repo': 'test', + 'namespace': None, + 'username': 'pingou'}) + + tests.add_readme_git_repo( + os.path.join(self.path, "repos", "forks", "pingou", "test.git")) + project = pagure.lib.query.get_authorized_project( + self.session, 'test') + fork = pagure.lib.query.get_authorized_project( + self.session, + 'test', + user='pingou', + ) + + tests.create_tokens(self.session) + tests.create_tokens_acl(self.session) + + req = pagure.lib.query.new_pull_request( + session=self.session, + repo_from=fork, + branch_from='master', + repo_to=project, + branch_to='master', + title='test pull-request', + user='pingou', + ) + self.session.commit() + self.assertEqual(req.id, 1) + self.assertEqual(req.title, 'test pull-request') + + # Assert the PR is open + self.session = pagure.lib.query.create_session(self.dbpath) + project = pagure.lib.query.get_authorized_project( + self.session, 'test') + self.assertEqual(len(project.requests), 1) + self.assertEqual(project.requests[0].status, "Open") + # Check how the PR renders in the API and the UI + output = self.app.get('/api/0/test/pull-request/1') + self.assertEqual(output.status_code, 200) + output = self.app.get('/test/pull-request/1') + self.assertEqual(output.status_code, 200) + + def test_api_assign_pr_invalid_project_namespace(self): + """ Test api_pull_request_assign method when the project doesn't exist. + """ + + headers = {'Authorization': 'token aaabbbcccddd'} + + # Valid token, wrong project + output = self.app.post( + '/api/0/somenamespace/test3/pull-request/1/assign', headers=headers) + self.assertEqual(output.status_code, 401) + data = json.loads(output.get_data(as_text=True)) + self.assertDictEqual( + data, + {'error': 'Invalid or expired token. Please visit ' + 'http://localhost.localdomain/settings#api-keys to get or renew your ' + 'API token.', + 'error_code': 'EINVALIDTOK'} + + ) + + def test_api_assign_pr_invalid_project(self): + """ Test api_pull_request_assign method when the project doesn't exist. + """ + + headers = {'Authorization': 'token aaabbbcccddd'} + + # Invalid project + output = self.app.post('/api/0/foo/pull-request/1/assign', headers=headers) + self.assertEqual(output.status_code, 404) + data = json.loads(output.get_data(as_text=True)) + self.assertDictEqual( + data, + { + "error": "Project not found", + "error_code": "ENOPROJECT", + } + ) + + def test_api_assign_pr_invalid_project_token(self): + """ Test api_pull_request_assign method when the token doesn't correspond + to the project. + """ + + headers = {'Authorization': 'token aaabbbcccddd'} + + # Valid token, wrong project + output = self.app.post('/api/0/test2/pull-request/1/assign', headers=headers) + self.assertEqual(output.status_code, 401) + data = json.loads(output.get_data(as_text=True)) + self.assertEqual(sorted(data.keys()), ['error', 'error_code']) + self.assertEqual( + pagure.api.APIERROR.EINVALIDTOK.value, data['error']) + self.assertEqual( + pagure.api.APIERROR.EINVALIDTOK.name, data['error_code']) + + def test_api_assign_pr_invalid_pr(self): + """ Test api_pull_request_assign method when asking for an invalid PR + """ + + headers = {'Authorization': 'token aaabbbcccddd'} + + # No input + output = self.app.post('/api/0/test/pull-request/404/assign', headers=headers) + self.assertEqual(output.status_code, 404) + data = json.loads(output.get_data(as_text=True)) + self.assertDictEqual( + data, + {'error': 'Pull-Request not found', 'error_code': 'ENOREQ'} + ) + + def test_api_assign_pr_no_input(self): + """ Test api_pull_request_assign method when no input is specified + """ + + headers = {'Authorization': 'token aaabbbcccddd'} + + # No input + output = self.app.post('/api/0/test/pull-request/1/assign', headers=headers) + self.assertEqual(output.status_code, 200) + data = json.loads(output.get_data(as_text=True)) + self.assertDictEqual( + data, + {'message': 'Nothing to change'} + ) + + def test_api_assign_pr_assigned(self): + """ Test api_pull_request_assign method when with valid input + """ + + headers = {'Authorization': 'token aaabbbcccddd'} + + data = { + 'assignee': 'pingou', + } + + # Valid request + output = self.app.post( + '/api/0/test/pull-request/1/assign', data=data, headers=headers) + self.assertEqual(output.status_code, 200) + data = json.loads(output.get_data(as_text=True)) + self.assertDictEqual( + data, + {'message': 'Request assigned'} + ) + + def test_api_assign_pr_unassigned(self): + """ Test api_pull_request_assign method when unassigning + """ + self.test_api_assign_pr_assigned() + + headers = {'Authorization': 'token aaabbbcccddd'} + data = {} + + # Un-assign + output = self.app.post( + '/api/0/test/pull-request/1/assign', data=data, headers=headers) + self.assertEqual(output.status_code, 200) + data = json.loads(output.get_data(as_text=True)) + self.assertDictEqual( + data, + {'message': 'Request assignee reset'} + ) + + def test_api_assign_pr_unassigned_twice(self): + """ Test api_pull_request_assign method when unassigning + """ + self.test_api_assign_pr_unassigned() + headers = {'Authorization': 'token aaabbbcccddd'} + data = {'assignee': None} + + # Un-assign + output = self.app.post( + '/api/0/test/pull-request/1/assign', data=data, headers=headers) + self.assertEqual(output.status_code, 200) + data = json.loads(output.get_data(as_text=True)) + self.assertDictEqual( + data, + {'message': 'Nothing to change'} + ) + + def test_api_assign_pr_unassigned_empty_string(self): + """ Test api_pull_request_assign method when unassigning with an + empty string + """ + self.test_api_assign_pr_assigned() + + headers = {'Authorization': 'token aaabbbcccddd'} + + # Un-assign + data = {'assignee': ''} + output = self.app.post( + '/api/0/test/pull-request/1/assign', data=data, headers=headers) + self.assertEqual(output.status_code, 200) + data = json.loads(output.get_data(as_text=True)) + self.assertDictEqual( + data, + {'message': 'Request assignee reset'} + ) + + +if __name__ == '__main__': + unittest.main(verbosity=2) diff --git a/tests/test_pagure_lib.py b/tests/test_pagure_lib.py index 84a264f..d0a613d 100644 --- a/tests/test_pagure_lib.py +++ b/tests/test_pagure_lib.py @@ -2998,7 +2998,7 @@ class PagureLibtests(tests.Modeltests): assignee=None, user='foo', ) - self.assertEqual(msg, 'Request reset') + self.assertEqual(msg, 'Request assignee reset') # Try resetting again msg = pagure.lib.query.add_pull_request_assignee( @@ -5534,6 +5534,7 @@ foo bar 'issue_update_custom_fields', 'issue_update_milestone', 'modify_project', + 'pull_request_assign', 'pull_request_close', 'pull_request_comment', 'pull_request_create', @@ -5541,6 +5542,7 @@ foo bar 'pull_request_merge', 'pull_request_rebase', 'pull_request_subscribe', + 'pull_request_update', 'update_watch_status', ] )