From 5a90176032f5369177a96a2177d15299f13bd0e7 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Mar 04 2019 13:24:27 +0000 Subject: Add a new API endpoint allowing to update an existing PR This endpoint allows to update the title and initial_comment of an existing PR. Fixes https://pagure.io/pagure/issue/4111 Signed-off-by: Pierre-Yves Chibon --- diff --git a/pagure/api/__init__.py b/pagure/api/__init__.py index 7630b9c..47d08fa 100644 --- a/pagure/api/__init__.py +++ b/pagure/api/__init__.py @@ -567,6 +567,7 @@ def api(): ) 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_pull_request_update_doc = load_doc(fork.api_pull_request_update) api_version_doc = load_doc(api_version) api_whoami_doc = load_doc(api_whoami) @@ -633,6 +634,7 @@ def api(): api_pull_request_add_comment_doc, api_pull_request_add_flag_doc, api_pull_request_assign_doc, + api_pull_request_update_doc, ], users=[ api_users_doc, diff --git a/pagure/api/fork.py b/pagure/api/fork.py index 4dea521..4de2d35 100644 --- a/pagure/api/fork.py +++ b/pagure/api/fork.py @@ -378,6 +378,131 @@ def api_pull_request_view(repo, requestid, username=None, namespace=None): return jsonout +@API.route("//pull-request/", methods=["POST"]) +@API.route( + "///pull-request/", methods=["POST"] +) +@API.route( + "/fork///pull-request/", methods=["POST"] +) +@API.route( + "/fork////pull-request/", + methods=["POST"], +) +@api_login_required(acls=["pull_request_update"]) +@api_method +def api_pull_request_update(repo, requestid, username=None, namespace=None): + """ + Update pull-request information + ------------------------------- + Update the title and initial comment of an existing pull-request. + + :: + + POST /api/0//pull-request/ + POST /api/0///pull-request/ + + :: + + POST /api/0/fork///pull-request/ + POST /api/0/fork////pull-request/ + + + Input + ^^^^^ + + +---------------------+--------+-------------+-----------------------------+ + | Key | Type | Optionality | Description | + +=====================+========+=============+=============================+ + | ``title`` | string | Mandatory | | The title to give to the | + | | | | pull-request | + +---------------------+--------+-------------+-----------------------------+ + | ``initial_comment`` | string | Optional | | The initial comment or | + | | | | description of the | + | | | | pull-request | + +---------------------+--------+-------------+-----------------------------+ + + Sample response + ^^^^^^^^^^^^^^^ + + :: + + { + "assignee": null, + "branch": "master", + "branch_from": "master", + "closed_at": null, + "closed_by": null, + "comments": [], + "commit_start": null, + "commit_stop": null, + "date_created": "1431414800", + "id": 1, + "project": { + "close_status": [], + "custom_keys": [], + "date_created": "1431414800", + "description": "test project #1", + "id": 1, + "name": "test", + "parent": null, + "user": { + "fullname": "PY C", + "name": "pingou" + } + }, + "repo_from": { + "date_created": "1431414800", + "description": "test project #1", + "id": 1, + "name": "test", + "parent": null, + "user": { + "fullname": "PY C", + "name": "pingou" + } + }, + "status": "Open", + "title": "test pull-request", + "uid": "1431414800", + "updated_on": "1431414800", + "user": { + "fullname": "PY C", + "name": "pingou" + } + } + + """ # noqa + + 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.RequestPullForm(csrf_enabled=False) + if not form.validate_on_submit(): + raise pagure.exceptions.APIError( + 400, error_code=APIERROR.EINVALIDREQ, errors=form.errors + ) + else: + request.title = form.title.data.strip() + request.initial_comment = form.initial_comment.data.strip() + flask.g.session.add(request) + try: + flask.g.session.commit() + except SQLAlchemyError as err: # pragma: no cover + flask.g.session.rollback() + _log.exception(err) + raise pagure.exceptions.APIError(400, error_code=APIERROR.EDBERROR) + + request = _get_request(repo, requestid) + jsonout = flask.jsonify(request.to_json(public=True, api=True)) + return jsonout + + @API.route("//pull-request//merge", methods=["POST"]) @API.route( "///pull-request//merge", methods=["POST"] diff --git a/tests/test_pagure_flask_api_fork_update.py b/tests/test_pagure_flask_api_fork_update.py new file mode 100644 index 0000000..aa19f08 --- /dev/null +++ b/tests/test_pagure_flask_api_fork_update.py @@ -0,0 +1,461 @@ +# -*- 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 PagureFlaskApiForkUpdatetests(tests.SimplePagureTest): + """ Tests for the flask API of pagure for updating 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(PagureFlaskApiForkUpdatetests, 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_pull_request_update_invalid_project_namespace(self): + """ Test api_pull_request_update 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', 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_pull_request_update_invalid_project(self): + """ Test api_pull_request_update method when the project doesn't exist. + """ + + headers = {'Authorization': 'token aaabbbcccddd'} + + # Invalid project + output = self.app.post('/api/0/foo/pull-request/1', 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_pull_request_update_invalid_project_token(self): + """ Test api_pull_request_update 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', 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_pull_request_update_invalid_pr(self): + """ Test api_assign_pull_request method when asking for an invalid PR + """ + + headers = {'Authorization': 'token aaabbbcccddd'} + + # Invalid PR id + output = self.app.post('/api/0/test/pull-request/404', 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_pull_request_update_no_input(self): + """ Test api_assign_pull_request method when no input is specified + """ + + headers = {'Authorization': 'token aaabbbcccddd'} + + # No input + output = self.app.post('/api/0/test/pull-request/1', headers=headers) + self.assertEqual(output.status_code, 400) + data = json.loads(output.get_data(as_text=True)) + self.assertDictEqual( + data, + { + 'error': 'Invalid or incomplete input submitted', + 'error_code': 'EINVALIDREQ', + 'errors': {'title': ['This field is required.']} + } + ) + + def test_api_pull_request_update_insufficient_input(self): + """ Test api_assign_pull_request method when no input is specified + """ + + headers = {'Authorization': 'token aaabbbcccddd'} + data = {'initial_comment': 'will not work'} + + # Missing the required title field + output = self.app.post('/api/0/test/pull-request/1', data=data, headers=headers) + self.assertEqual(output.status_code, 400) + data = json.loads(output.get_data(as_text=True)) + self.assertDictEqual( + data, + { + 'error': 'Invalid or incomplete input submitted', + 'error_code': 'EINVALIDREQ', + 'errors': {'title': ['This field is required.']} + } + ) + + def test_api_pull_request_update_edited(self): + """ Test api_assign_pull_request method when with valid input + """ + + headers = {'Authorization': 'token aaabbbcccddd'} + + data = { + 'title': 'edited test PR', + 'initial_comment': 'Edited initial comment', + } + + # Valid request + output = self.app.post( + '/api/0/test/pull-request/1', data=data, headers=headers) + self.assertEqual(output.status_code, 200) + data = json.loads(output.get_data(as_text=True)) + # Hard-code all the values that will change from a test to another + # because either random or time-based + data['date_created'] = '1551276260' + data['last_updated'] = '1551276261' + data['updated_on'] = '1551276260' + data['commit_start'] = '5f5d609db65d447f77ba00e25afd17ba5053344b' + data['commit_stop'] = '5f5d609db65d447f77ba00e25afd17ba5053344b' + data['project']['date_created'] = '1551276259' + data['project']['date_modified'] = '1551276259' + data['repo_from']['date_created'] = '1551276259' + data['repo_from']['date_modified'] = '1551276259' + data['repo_from']['parent']['date_created'] = '1551276259' + data['repo_from']['parent']['date_modified'] = '1551276259' + data['uid'] = 'a2bddecc8ea548e88c22a0df77670092' + self.assertDictEqual( + data, + { + 'assignee': None, + 'branch': 'master', + 'branch_from': 'master', + 'cached_merge_status': 'unknown', + 'closed_at': None, + 'closed_by': None, + 'comments': [], + 'commit_start': '5f5d609db65d447f77ba00e25afd17ba5053344b', + 'commit_stop': '5f5d609db65d447f77ba00e25afd17ba5053344b', + 'date_created': '1551276260', + 'id': 1, + 'initial_comment': 'Edited initial comment', + 'last_updated': '1551276261', + 'project': {'access_groups': {'admin': [], 'commit': [], 'ticket': []}, + 'access_users': {'admin': [], + 'commit': [], + 'owner': ['pingou'], + 'ticket': []}, + 'close_status': ['Invalid', + 'Insufficient data', + 'Fixed', + 'Duplicate'], + 'custom_keys': [], + 'date_created': '1551276259', + 'date_modified': '1551276259', + 'description': 'test project #1', + 'fullname': 'test', + 'id': 1, + 'milestones': {}, + 'name': 'test', + 'namespace': None, + 'parent': None, + 'priorities': {}, + 'tags': [], + 'url_path': 'test', + 'user': {'fullname': 'PY C', 'name': 'pingou'}}, + 'remote_git': None, + 'repo_from': {'access_groups': {'admin': [], 'commit': [], 'ticket': []}, + 'access_users': {'admin': [], + 'commit': [], + 'owner': ['pingou'], + 'ticket': []}, + 'close_status': [], + 'custom_keys': [], + 'date_created': '1551276259', + 'date_modified': '1551276259', + 'description': 'test project #1', + 'fullname': 'forks/pingou/test', + 'id': 4, + 'milestones': {}, + 'name': 'test', + 'namespace': None, + 'parent': {'access_groups': {'admin': [], + 'commit': [], + 'ticket': []}, + 'access_users': {'admin': [], + 'commit': [], + 'owner': ['pingou'], + 'ticket': []}, + 'close_status': ['Invalid', + 'Insufficient data', + 'Fixed', + 'Duplicate'], + 'custom_keys': [], + 'date_created': '1551276259', + 'date_modified': '1551276259', + 'description': 'test project #1', + 'fullname': 'test', + 'id': 1, + 'milestones': {}, + 'name': 'test', + 'namespace': None, + 'parent': None, + 'priorities': {}, + 'tags': [], + 'url_path': 'test', + 'user': {'fullname': 'PY C', 'name': 'pingou'}}, + 'priorities': {}, + 'tags': [], + 'url_path': 'fork/pingou/test', + 'user': {'fullname': 'PY C', 'name': 'pingou'}}, + 'status': 'Open', + 'tags': [], + 'threshold_reached': None, + 'title': 'edited test PR', + 'uid': 'a2bddecc8ea548e88c22a0df77670092', + 'updated_on': '1551276260', + 'user': {'fullname': 'PY C', 'name': 'pingou'} + } + ) + + def test_api_pull_request_update_edited_no_comment(self): + """ Test api_assign_pull_request method when with valid input + """ + + headers = {'Authorization': 'token aaabbbcccddd'} + + data = { + 'title': 'edited test PR', + } + + # Valid request + output = self.app.post( + '/api/0/test/pull-request/1', data=data, headers=headers) + self.assertEqual(output.status_code, 200) + data = json.loads(output.get_data(as_text=True)) + # Hard-code all the values that will change from a test to another + # because either random or time-based + data['date_created'] = '1551276260' + data['last_updated'] = '1551276261' + data['updated_on'] = '1551276260' + data['commit_start'] = '5f5d609db65d447f77ba00e25afd17ba5053344b' + data['commit_stop'] = '5f5d609db65d447f77ba00e25afd17ba5053344b' + data['project']['date_created'] = '1551276259' + data['project']['date_modified'] = '1551276259' + data['repo_from']['date_created'] = '1551276259' + data['repo_from']['date_modified'] = '1551276259' + data['repo_from']['parent']['date_created'] = '1551276259' + data['repo_from']['parent']['date_modified'] = '1551276259' + data['uid'] = 'a2bddecc8ea548e88c22a0df77670092' + self.assertDictEqual( + data, + { + 'assignee': None, + 'branch': 'master', + 'branch_from': 'master', + 'cached_merge_status': 'unknown', + 'closed_at': None, + 'closed_by': None, + 'comments': [], + 'commit_start': '5f5d609db65d447f77ba00e25afd17ba5053344b', + 'commit_stop': '5f5d609db65d447f77ba00e25afd17ba5053344b', + 'date_created': '1551276260', + 'id': 1, + 'initial_comment': '', + 'last_updated': '1551276261', + 'project': {'access_groups': {'admin': [], 'commit': [], 'ticket': []}, + 'access_users': {'admin': [], + 'commit': [], + 'owner': ['pingou'], + 'ticket': []}, + 'close_status': ['Invalid', + 'Insufficient data', + 'Fixed', + 'Duplicate'], + 'custom_keys': [], + 'date_created': '1551276259', + 'date_modified': '1551276259', + 'description': 'test project #1', + 'fullname': 'test', + 'id': 1, + 'milestones': {}, + 'name': 'test', + 'namespace': None, + 'parent': None, + 'priorities': {}, + 'tags': [], + 'url_path': 'test', + 'user': {'fullname': 'PY C', 'name': 'pingou'}}, + 'remote_git': None, + 'repo_from': {'access_groups': {'admin': [], 'commit': [], 'ticket': []}, + 'access_users': {'admin': [], + 'commit': [], + 'owner': ['pingou'], + 'ticket': []}, + 'close_status': [], + 'custom_keys': [], + 'date_created': '1551276259', + 'date_modified': '1551276259', + 'description': 'test project #1', + 'fullname': 'forks/pingou/test', + 'id': 4, + 'milestones': {}, + 'name': 'test', + 'namespace': None, + 'parent': {'access_groups': {'admin': [], + 'commit': [], + 'ticket': []}, + 'access_users': {'admin': [], + 'commit': [], + 'owner': ['pingou'], + 'ticket': []}, + 'close_status': ['Invalid', + 'Insufficient data', + 'Fixed', + 'Duplicate'], + 'custom_keys': [], + 'date_created': '1551276259', + 'date_modified': '1551276259', + 'description': 'test project #1', + 'fullname': 'test', + 'id': 1, + 'milestones': {}, + 'name': 'test', + 'namespace': None, + 'parent': None, + 'priorities': {}, + 'tags': [], + 'url_path': 'test', + 'user': {'fullname': 'PY C', 'name': 'pingou'}}, + 'priorities': {}, + 'tags': [], + 'url_path': 'fork/pingou/test', + 'user': {'fullname': 'PY C', 'name': 'pingou'}}, + 'status': 'Open', + 'tags': [], + 'threshold_reached': None, + 'title': 'edited test PR', + 'uid': 'a2bddecc8ea548e88c22a0df77670092', + 'updated_on': '1551276260', + 'user': {'fullname': 'PY C', 'name': 'pingou'} + } + ) + +if __name__ == '__main__': + unittest.main(verbosity=2)