diff --git a/pagure/api/fork.py b/pagure/api/fork.py index e676e4d..9451f94 100644 --- a/pagure/api/fork.py +++ b/pagure/api/fork.py @@ -659,7 +659,11 @@ def api_pull_request_rebase(repo, requestid, username=None, namespace=None): raise pagure.exceptions.APIError(403, error_code=APIERROR.ENOPRCLOSE) task = pagure.lib.tasks.rebase_pull_request.delay( - repo.name, namespace, username, requestid, flask.g.fas_user.username + repo.name, + namespace, + username, + requestid, + user_rebaser=flask.g.fas_user.username, ) output = {"message": "Rebasing queued", "taskid": task.id} diff --git a/pagure/lib/git.py b/pagure/lib/git.py index ae5a971..f3415b3 100644 --- a/pagure/lib/git.py +++ b/pagure/lib/git.py @@ -1514,7 +1514,12 @@ def merge_pull_request(session, request, username, domerge=True): # Update the start and stop commits in the DB, one last time diff_commits = diff_pull_request( - session, request, fork_obj, new_repo, with_diff=False + session, + request, + fork_obj, + new_repo, + with_diff=False, + username=username, ) _log.info(" %s commit to merge", len(diff_commits)) @@ -2097,7 +2102,13 @@ def get_diff_info(repo_obj, orig_repo, branch_from, branch_to, prid=None): def diff_pull_request( - session, request, repo_obj, orig_repo, with_diff=True, notify=True + session, + request, + repo_obj, + orig_repo, + with_diff=True, + notify=True, + username=None, ): """ Returns the diff and the list of commits between the two git repos mentionned in the given pull-request. @@ -2109,9 +2120,11 @@ def diff_pull_request( :arg orig_repo: The pygit2.Repository object of the second git repo :arg with_diff: A boolean on whether to return the diff with the list of commits (or just the list of commits) + :arg username: The username of the user diffing the pull-request """ + _log.debug("pagure.lib.git.diff_pull_request, started") diff = None diff_commits = [] diff, diff_commits, _ = get_diff_info( @@ -2121,8 +2134,10 @@ def diff_pull_request( request.branch, prid=request.id, ) + _log.debug("pagure.lib.git.diff_pull_request, diff done") if request.status == "Open" and diff_commits: + _log.debug("pagure.lib.git.diff_pull_request, PR open and with a diff") first_commit = diff_commits[-1] # Check if we can still rely on the merge_status commenttext = None @@ -2163,6 +2178,9 @@ def diff_pull_request( request.commit_stop = diff_commits[0].oid.hex session.add(request) session.commit() + _log.debug( + "pagure.lib.git.diff_pull_request, commenttext: %s", commenttext + ) pagure.lib.tasks.sync_pull_ref.delay( request.project.name, @@ -2185,6 +2203,12 @@ def diff_pull_request( agent="pagure", ), ) + _log.debug( + "pagure.lib.git.diff_pull_request: adding notification: %s" + "as user: %s", + commenttext, + username or request.user.username, + ) pagure.lib.query.add_pull_request_comment( session, request, @@ -2193,7 +2217,7 @@ def diff_pull_request( filename=None, row=None, comment="%s" % commenttext, - user=request.user.username, + user=username or request.user.username, notify=False, notification=True, ) diff --git a/pagure/lib/tasks.py b/pagure/lib/tasks.py index 50a9b9a..14d0604 100644 --- a/pagure/lib/tasks.py +++ b/pagure/lib/tasks.py @@ -726,6 +726,7 @@ def rebase_pull_request( ) pagure.lib.git.rebase_pull_request(request, user_rebaser) + update_pull_request(request.uid, username=user_rebaser) # Schedule refresh of all opened PRs pagure.lib.query.reset_status_pull_request(session, request.project) @@ -870,7 +871,7 @@ def sync_pull_ref(self, session, name, namespace, user, requestid): @conn.task(queue=pagure_config.get("FAST_CELERY_QUEUE", None), bind=True) @pagure_task -def update_pull_request(self, session, pr_uid): +def update_pull_request(self, session, pr_uid, username=None): """ Updates a pull-request in the DB once a commit was pushed to it in git. """ @@ -886,7 +887,10 @@ def update_pull_request(self, session, pr_uid): try: pagure.lib.git.merge_pull_request( - session=session, request=request, username=None, domerge=False + session=session, + request=request, + username=username, + domerge=False, ) except pagure.exceptions.PagureException as err: _log.debug(err) diff --git a/tests/test_pagure_flask_rebase.py b/tests/test_pagure_flask_rebase.py index f651d7e..b1dabd8 100644 --- a/tests/test_pagure_flask_rebase.py +++ b/tests/test_pagure_flask_rebase.py @@ -45,7 +45,10 @@ class PagureRebasetests(tests.Modeltests): os.path.join(self.path, 'requests'), bare=True) tests.add_content_to_git( os.path.join(self.path, 'repos', 'test.git'), - branch='test', content="foobar") + branch='master', content="foobarbaz", filename="testfile") + tests.add_content_to_git( + os.path.join(self.path, 'repos', 'test.git'), + branch='test', content="foobar", filename="sources") tests.add_readme_git_repo( os.path.join(self.path, 'repos', 'test.git')) @@ -144,6 +147,22 @@ class PagureRebasetests(tests.Modeltests): user = tests.FakeUser(username='pingou') with tests.user_set(self.app.application, user): + # Get the merge status first so it's cached and can be refreshed + csrf_token = self.get_csrf() + data = {'requestid': self.request.uid, 'csrf_token': csrf_token} + output = self.app.post('/pv/pull-request/merge', data=data) + self.assertEqual(output.status_code, 200) + data = json.loads(output.get_data(as_text=True)) + self.assertEqual( + data, + { + u'code': u'MERGE', + u'message': u'The pull-request can be merged with ' + u'a merge commit', + u'short_code': u'With merge' + } + ) + output = self.app.post('/api/0/test/pull-request/1/rebase') self.assertEqual(output.status_code, 200) data = json.loads(output.get_data(as_text=True)) @@ -152,7 +171,7 @@ class PagureRebasetests(tests.Modeltests): {u'message': u'Pull-request rebased'} ) - data = {'requestid': self.request.uid, 'csrf_token': self.get_csrf()} + data = {'requestid': self.request.uid, 'csrf_token': csrf_token} output = self.app.post('/pv/pull-request/merge', data=data) self.assertEqual(output.status_code, 200) data = json.loads(output.get_data(as_text=True)) @@ -166,6 +185,75 @@ class PagureRebasetests(tests.Modeltests): } ) + output = self.app.get('/test/pull-request/1') + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + self.assertIn('rebased onto', output_text) + repo = pagure.lib.query._get_project(self.session, 'test') + self.assertEqual( + repo.requests[0].comments[0].user.username, 'pingou') + + def test_rebase_api_ui_logged_in_different_user(self): + """ Test the rebase PR API endpoint when logged in from the UI and + its outcome. """ + # Add 'foo' to the project 'test' so 'foo' can rebase the PR + repo = pagure.lib.query._get_project(self.session, 'test') + msg = pagure.lib.query.add_user_to_project( + session=self.session, + project=repo, + new_user='foo', + user='pingou', + ) + self.session.commit() + self.assertEqual(msg, 'User added') + + user = tests.FakeUser(username='foo') + with tests.user_set(self.app.application, user): + # Get the merge status first so it's cached and can be refreshed + csrf_token = self.get_csrf() + data = {'requestid': self.request.uid, 'csrf_token': csrf_token} + output = self.app.post('/pv/pull-request/merge', data=data) + self.assertEqual(output.status_code, 200) + data = json.loads(output.get_data(as_text=True)) + self.assertEqual( + data, + { + u'code': u'MERGE', + u'message': u'The pull-request can be merged with ' + u'a merge commit', + u'short_code': u'With merge' + } + ) + + output = self.app.post('/api/0/test/pull-request/1/rebase') + self.assertEqual(output.status_code, 200) + data = json.loads(output.get_data(as_text=True)) + self.assertEqual( + data, + {u'message': u'Pull-request rebased'} + ) + + data = {'requestid': self.request.uid, 'csrf_token': csrf_token} + output = self.app.post('/pv/pull-request/merge', data=data) + self.assertEqual(output.status_code, 200) + data = json.loads(output.get_data(as_text=True)) + self.assertEqual( + data, + { + u'code': u'FFORWARD', + u'message': u'The pull-request can be merged and ' + u'fast-forwarded', + u'short_code': u'Ok' + } + ) + + output = self.app.get('/test/pull-request/1') + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + self.assertIn('rebased onto', output_text) + repo = pagure.lib.query._get_project(self.session, 'test') + self.assertEqual(repo.requests[0].comments[0].user.username, 'foo') + def test_rebase_api_api_logged_in(self): """ Test the rebase PR API endpoint when using an API token and its outcome. """