From 6937b661505735ef6a04d81c1b40a435dd4b05c2 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Oct 03 2018 13:01:08 +0000 Subject: Display when a PR cannot be merged because of its review score Relates to https://pagure.io/pagure/issue/3701 Signed-off-by: Pierre-Yves Chibon --- diff --git a/pagure/internal/__init__.py b/pagure/internal/__init__.py index d1942e3..00b911c 100644 --- a/pagure/internal/__init__.py +++ b/pagure/internal/__init__.py @@ -215,6 +215,21 @@ def mergeable_request_pull(): response.status_code = 404 return response + threshold = request.project.settings.get( + "Minimum_score_to_merge_pull-request", -1 + ) + if threshold > 0 and int(request.score) < int(threshold): + response = flask.jsonify( + { + "code": "CONFLICTS", + "message": "Pull-Request does not meet the minimal " + "number of review required: %s/%s" + % (request.score, threshold), + } + ) + response.status_code = 400 + return response + merge_status = request.merge_status if not merge_status or force: try: diff --git a/tests/test_pagure_flask_internal.py b/tests/test_pagure_flask_internal.py index c6ab1d5..338ef0d 100644 --- a/tests/test_pagure_flask_internal.py +++ b/tests/test_pagure_flask_internal.py @@ -1160,6 +1160,141 @@ class PagureFlaskInternaltests(tests.Modeltests): js_data = json.loads(output.get_data(as_text=True)) self.assertDictEqual(js_data, exp) + @patch('pagure.lib.notify.send_email') + def test_mergeable_request_pull_minimum_score(self, send_email): + """ Test the mergeable_request_pull endpoint when the changes can + be merged with a merge FF, but project settings enforces vote on + the PR. + """ + send_email.return_value = True + + # Create a git repo to play with + + origgitrepo = os.path.join(self.path, 'repos', 'test.git') + self.assertFalse(os.path.exists(origgitrepo)) + os.makedirs(origgitrepo) + orig_repo = pygit2.init_repository(origgitrepo, bare=True) + os.makedirs(os.path.join(self.path, 'repos_tmp')) + gitrepo = os.path.join(self.path, 'repos_tmp', 'test.git') + repo = pygit2.clone_repository(origgitrepo, gitrepo) + + # Create a file in that git repo + with open(os.path.join(gitrepo, 'sources'), 'w') as stream: + stream.write('foo\n bar') + repo.index.add('sources') + repo.index.write() + + # Commits the files added + tree = repo.index.write_tree() + author = pygit2.Signature( + 'Alice Author', 'alice@authors.tld') + committer = pygit2.Signature( + 'Cecil Committer', 'cecil@committers.tld') + repo.create_commit( + 'refs/heads/master', # the name of the reference to update + author, + committer, + 'Add sources file for testing', + # binary string representing the tree object ID + tree, + # list of binary strings representing parents of the new commit + [] + ) + + first_commit = repo.revparse_single('HEAD') + refname = 'refs/heads/master:refs/heads/master' + ori_remote = repo.remotes[0] + PagureRepo.push(ori_remote, refname) + + # Edit the sources file again + with open(os.path.join(gitrepo, 'sources'), 'w') as stream: + stream.write('foo\n bar\nbaz\n boose') + repo.index.add('sources') + repo.index.write() + + # Commits the files added + tree = repo.index.write_tree() + author = pygit2.Signature( + 'Alice Author', 'alice@authors.tld') + committer = pygit2.Signature( + 'Cecil Committer', 'cecil@committers.tld') + repo.create_commit( + 'refs/heads/feature', # the name of the reference to update + author, + committer, + 'Add baz and boose to the sources\n\n There are more objects to ' + 'consider', + # binary string representing the tree object ID + tree, + # list of binary strings representing parents of the new commit + [first_commit.oid.hex] + ) + refname = 'refs/heads/feature:refs/heads/feature' + ori_remote = repo.remotes[0] + PagureRepo.push(ori_remote, refname) + + # Create a PR for these changes + tests.create_projects(self.session) + project = pagure.lib.get_authorized_project(self.session, 'test') + req = pagure.lib.new_pull_request( + session=self.session, + repo_from=project, + branch_from='feature', + repo_to=project, + branch_to='master', + title='PR from the feature branch', + user='pingou', + ) + settings = {'Minimum_score_to_merge_pull-request': 2} + project.settings = settings + self.session.commit() + self.assertEqual(req.id, 1) + self.assertEqual(req.title, 'PR from the feature branch') + + # Check if the PR can be merged + data = {} + + user = tests.FakeUser() + user.username = 'pingou' + with tests.user_set(self.app.application, user): + output = self.app.get('/test/adduser') + csrf_token = output.get_data(as_text=True).split( + 'name="csrf_token" type="hidden" value="')[1].split('">')[0] + + # With all the desired information + project = pagure.lib.get_authorized_project(self.session, 'test') + data = { + 'csrf_token': csrf_token, + 'requestid': project.requests[0].uid, + 'force': True, + } + output = self.app.post('/pv/pull-request/merge', data=data) + self.assertEqual(output.status_code, 400) + exp = { + "code": "CONFLICTS", + "message": "Pull-Request does not meet the minimal number " + "of review required: 0/2" + } + + js_data = json.loads(output.get_data(as_text=True)) + self.assertDictEqual(js_data, exp) + + # Asking a second time will trigger the cache + data = { + 'csrf_token': csrf_token, + 'requestid': project.requests[0].uid, + } + output = self.app.post('/pv/pull-request/merge', data=data) + self.assertEqual(output.status_code, 400) + exp = { + "code": "CONFLICTS", + "message": "Pull-Request does not meet the minimal number " + "of review required: 0/2" + } + + js_data = json.loads(output.get_data(as_text=True)) + self.assertDictEqual(js_data, exp) + @patch('pagure.lib.notify.send_email', MagicMock(return_value=True)) @patch( 'pagure.lib.git.merge_pull_request',