diff --git a/doc/configuration.rst b/doc/configuration.rst index 7bfa747..76e444d 100644 --- a/doc/configuration.rst +++ b/doc/configuration.rst @@ -676,6 +676,19 @@ or projects for example. Defaults to: ``50``. +PR_TARGET_MATCHING_BRANCH +~~~~~~~~~~~~~~~~~~~~~~~~~ + +If set to ``True``, the default target branch for all pull requests in UI +is the branch that is longest substring of the branch that the pull request +is created from. For example, a ``mybranch`` branch in original repo will +be the default target of a pull request from branch ``mybranch-feature-1`` +in a fork when opening a new pull request. If this is set to ``False``, +the default branch of the repo will be the default target of all pull requests. + +Defaults to: ``False``. + + SMTP_SERVER ~~~~~~~~~~~ diff --git a/pagure/default_config.py b/pagure/default_config.py index e828816..8795758 100644 --- a/pagure/default_config.py +++ b/pagure/default_config.py @@ -82,6 +82,11 @@ LOCAL_SSH_KEY = True # Enable / Disable deploy keys DEPLOY_KEY = True +# Set to True if default target branch for all PRs in UI +# should be the branch that is longest substring of the branch +# that the PR is to be created from +PR_TARGET_MATCHING_BRANCH = False + # Enables / Disables showing all the projects by default on the front page SHOW_PROJECTS_INDEX = ['repos', 'myrepos', 'myforks'] diff --git a/pagure/internal/__init__.py b/pagure/internal/__init__.py index 5282f64..9f2a7ed 100644 --- a/pagure/internal/__init__.py +++ b/pagure/internal/__init__.py @@ -297,16 +297,29 @@ def get_pull_request_ready_branch(): branches = {} if not repo_obj.is_empty and len(repo_obj.listall_branches()) > 0: - if not parent_repo_obj.is_empty \ - and not parent_repo_obj.head_is_unborn: - try: - compare_branch = repo_obj.head.shorthand - except pygit2.GitError: - compare_branch = None - else: - compare_branch = None - for branchname in repo_obj.listall_branches(): + compare_branch = None + if not parent_repo_obj.is_empty \ + and not parent_repo_obj.head_is_unborn: + try: + if pagure.config.config.get('PR_TARGET_MATCHING_BRANCH', + False): + # find parent branch which is the longest substring of + # branch that we're processing + compare_branch = '' + for parent_branch in parent_repo_obj.branches: + if not repo.is_fork \ + and branchname == parent_branch: + continue + if branchname.startswith(parent_branch) and \ + len(parent_branch) > len(compare_branch): + compare_branch = parent_branch + compare_branch = compare_branch \ + or repo_obj.head.shorthand + else: + compare_branch = repo_obj.head.shorthand + except pygit2.GitError: + pass # let compare_branch be None # Do not compare a branch to itself if not repo.is_fork \ @@ -322,7 +335,10 @@ def get_pull_request_ready_branch(): pass if diff_commits: - branches[branchname] = [c.oid.hex for c in diff_commits] + branches[branchname] = { + 'commits': [c.oid.hex for c in diff_commits], + 'target_branch': compare_branch or 'master', + } prs = pagure.lib.search_pull_requests( flask.g.session, diff --git a/pagure/templates/repo_branches.html b/pagure/templates/repo_branches.html index d50894b..c4a25b0 100644 --- a/pagure/templates/repo_branches.html +++ b/pagure/templates/repo_branches.html @@ -126,36 +126,39 @@ $(function() { if (res.code == 'OK'){ console.log(res.message); for (branch in res.message.new_branch){ + var nb_commits = res.message.new_branch[branch]['commits'] + var nb_target = res.message.new_branch[branch]['target_branch'] var url = "{{ url_for( 'ui_ns.new_request_pull', repo=repo.name, username=repo.user.user if repo.is_fork else None, namespace=repo.namespace, - branch_to=head or 'master', + branch_to='', branch_from='') }}"; + url = url.slice(0, -2) + nb_target + '..' + branch {% if repo.is_fork %} html2 = ' \ Open Pull Request \ + href="' + url + '" title="' + branch +' contains ' + + nb_commits.length + ' commit not in the upstream project ' + + nb_target + ' branch. Click to create new PR now.' + + '"> Open Pull Request \ '; {% else %} html2 = ' \ Open Pull Request \ + href="' + url + '" title="' + branch +' contains ' + + nb_commits.length + ' commit not in the ' + nb_target + + ' branch. Click to create new PR now.' + + '"> Open Pull Request \ '; {%endif%} var _b = branch.replace(/\./g, '\\.').replace('/', '__').replace('\+', '\\+'); $('#branch-' + _b + ' .branch_del').prepend(html2); $('[data-toggle="tooltip"]').tooltip({placement : 'bottom'}); - var commits_string = (res.message.new_branch[branch].length>1) ? " commits" : " commit" - $('#branch-' + _b + ' .commits_ahead_label').append(res.message.new_branch[branch].length+commits_string+' ahead'); + var commits_string = (nb_commits.length > 1) ? " commits" : " commit" + $('#branch-' + _b + ' .commits_ahead_label').append(nb_commits.length + commits_string + ' ahead'); } diff --git a/pagure/templates/repo_info.html b/pagure/templates/repo_info.html index 44a755a..5633ee6 100644 --- a/pagure/templates/repo_info.html +++ b/pagure/templates/repo_info.html @@ -366,39 +366,39 @@ $(function() { success: function(res) { if (res.code == 'OK'){ for (branch in res.message.new_branch){ + var nb_commits = res.message.new_branch[branch]['commits'] + var nb_target = res.message.new_branch[branch]['target_branch'] var url = "{{ url_for( 'ui_ns.new_request_pull', repo=repo.name, username=repo.user.user if repo.is_fork else None, namespace=repo.namespace, - branch_to=head or 'master', + branch_to='', branch_from='') }}"; + url = url.slice(0, -2) + nb_target + '..' + branch html = ''; /*$($('.bodycontent').find('.row').children()[0]).before(html);*/ {% if repo.is_fork %} html2 = ' \ New PR \ + href="' + url + '"title="' + branch +' contains ' + nb_commits.length + ' \ + commit not in the upstream project ' + + nb_target + ' branch. Click to create new PR now.'+'"> New PR \ '; {% else %} html2 = ' \ New PR \ + href="' + url + '" title="' + branch +' contains ' + nb_commits.length + ' \ + commit not in the ' + nb_target + ' branch. Click to create new PR now.' \ + +'"> New PR \ '; {%endif%} var _b = branch.replace(/\./g, '\\.').replace('/', '__').replace('\+', '\\+'); diff --git a/pagure/templates/repo_master.html b/pagure/templates/repo_master.html index 64516c4..b815ff5 100644 --- a/pagure/templates/repo_master.html +++ b/pagure/templates/repo_master.html @@ -429,8 +429,9 @@ $(function() { repo=repo.name, username=g.fas_user.username, namespace=repo.namespace, - branch_to=head or 'master', - branch_from='') }}" + branch; + branch_to='', + branch_from='') }}"; + url = url.slice(0, -2) + res.message.new_branch[branch]['target_branch'] + '..' + branch; html = 'From ' + ' ' @@ -469,9 +470,10 @@ $(function() { repo=repo.name, username=repo.user.user if repo.is_fork else None, namespace=repo.namespace, - branch_to=head, + branch_to='', branch_from='') }}"; - html = '' + url = url.slice(0, -2) + res.message.new_branch[branch]['target_branch'] + '..' + branch; + html = '' + 'From ' + ' ' + '{{ repo.name }}' diff --git a/tests/test_pagure_flask_internal.py b/tests/test_pagure_flask_internal.py index 71d1468..8569070 100644 --- a/tests/test_pagure_flask_internal.py +++ b/tests/test_pagure_flask_internal.py @@ -1856,7 +1856,9 @@ class PagureFlaskInternaltests(tests.Modeltests): self.assertEqual(js_data['message']['branch_w_pr'], {}) self.assertListEqual( list(js_data['message']['new_branch']), ['feature']) - self.assertEqual(len(js_data['message']['new_branch']['feature']), 2) + self.assertEqual(len(js_data['message']['new_branch']['feature']['commits']), 2) + self.assertEqual( + js_data['message']['new_branch']['feature']['target_branch'], 'master') def test_get_pull_request_ready_branch_on_fork_no_parent_no_pr(self): '''Test the get_pull_request_ready_branch from the internal API on @@ -1982,7 +1984,138 @@ class PagureFlaskInternaltests(tests.Modeltests): self.assertEqual(js_data['message']['branch_w_pr'], {}) self.assertListEqual( list(js_data['message']['new_branch']), ['feature']) - self.assertEqual(len(js_data['message']['new_branch']['feature']), 2) + self.assertEqual(len(js_data['message']['new_branch']['feature']['commits']), 2) + self.assertEqual( + js_data['message']['new_branch']['feature']['target_branch'], 'master') + + def test_get_pull_request_ready_branch_matching_target_off(self): + '''Test the get_pull_request_ready_branch from the internal API on + a fork while PR_TARGET_MATCHING_BRANCH is False + ''' + tests.create_projects(self.session) + # make sure that head is not unborn + tests.add_content_git_repo( + os.path.join(self.path, 'repos', 'test.git'), + branch='master') + tests.add_content_git_repo( + os.path.join(self.path, 'repos', 'test.git'), + branch='feature') + + tests.create_projects_git( + os.path.join(self.path, 'repos'), bare=True) + tests.create_projects_git( + os.path.join(self.path, 'repos', 'forks', 'foo'), bare=True) + tests.add_content_git_repo( + os.path.join(self.path, 'repos', 'forks', 'foo', 'test.git'), + branch='feature') + + # Create foo's fork of the test project + item = pagure.lib.model.Project( + user_id=2, # foo + name='test', + is_fork=True, + parent_id=1, # test + description='test project #1', + hook_token='aaabbbcccddd', + ) + item.close_status = ['Invalid', 'Insufficient data', 'Fixed', 'Duplicate'] + self.session.add(item) + self.session.commit() + + # Get on with testing + user = tests.FakeUser() + user.username = 'pingou' + with tests.user_set(self.app.application, user): + csrf_token = self.get_csrf() + + # Query branches on the Ralph's fork + data = { + 'csrf_token': csrf_token, + 'repo': 'test', + 'repouser': 'foo', + } + output = self.app.post('/pv/pull-request/ready', data=data) + self.assertEqual(output.status_code, 200) + js_data = json.loads(output.get_data(as_text=True)) + self.assertEqual( + sorted(js_data.keys()), + ['code', 'message'] + ) + self.assertEqual(js_data['code'], 'OK') + self.assertListEqual( + sorted(js_data['message'].keys()), + ['branch_w_pr', 'new_branch']) + self.assertEqual(js_data['message']['branch_w_pr'], {}) + self.assertListEqual( + list(js_data['message']['new_branch']), ['feature']) + self.assertEqual(len(js_data['message']['new_branch']['feature']['commits']), 2) + self.assertEqual( + js_data['message']['new_branch']['feature']['target_branch'], 'master') + + @patch.dict('pagure.config.config', {'PR_TARGET_MATCHING_BRANCH': True}) + def test_get_pull_request_ready_branch_matching_target_on(self): + '''Test the get_pull_request_ready_branch from the internal API on + a fork while PR_TARGET_MATCHING_BRANCH is True + ''' + tests.create_projects(self.session) + # make sure that head is not unborn + tests.add_content_git_repo( + os.path.join(self.path, 'repos', 'test.git'), + branch='master') + tests.add_content_git_repo( + os.path.join(self.path, 'repos', 'test.git'), + branch='feature') + + tests.create_projects_git( + os.path.join(self.path, 'repos'), bare=True) + tests.create_projects_git( + os.path.join(self.path, 'repos', 'forks', 'foo'), bare=True) + tests.add_content_git_repo( + os.path.join(self.path, 'repos', 'forks', 'foo', 'test.git'), + branch='feature') + + # Create foo's fork of the test project + item = pagure.lib.model.Project( + user_id=2, # foo + name='test', + is_fork=True, + parent_id=1, # test + description='test project #1', + hook_token='aaabbbcccddd', + ) + item.close_status = ['Invalid', 'Insufficient data', 'Fixed', 'Duplicate'] + self.session.add(item) + self.session.commit() + + # Get on with testing + user = tests.FakeUser() + user.username = 'pingou' + with tests.user_set(self.app.application, user): + csrf_token = self.get_csrf() + + # Query branches on the Ralph's fork + data = { + 'csrf_token': csrf_token, + 'repo': 'test', + 'repouser': 'foo', + } + output = self.app.post('/pv/pull-request/ready', data=data) + self.assertEqual(output.status_code, 200) + js_data = json.loads(output.get_data(as_text=True)) + self.assertEqual( + sorted(js_data.keys()), + ['code', 'message'] + ) + self.assertEqual(js_data['code'], 'OK') + self.assertListEqual( + sorted(js_data['message'].keys()), + ['branch_w_pr', 'new_branch']) + self.assertEqual(js_data['message']['branch_w_pr'], {}) + self.assertListEqual( + list(js_data['message']['new_branch']), ['feature']) + self.assertEqual(len(js_data['message']['new_branch']['feature']['commits']), 1) + self.assertEqual( + js_data['message']['new_branch']['feature']['target_branch'], 'feature') if __name__ == '__main__':