diff --git a/rhodecode/api/tests/test_get_pull_request.py b/rhodecode/api/tests/test_get_pull_request.py --- a/rhodecode/api/tests/test_get_pull_request.py +++ b/rhodecode/api/tests/test_get_pull_request.py @@ -21,10 +21,10 @@ import pytest import urlobject -from pylons import url from rhodecode.api.tests.utils import ( build_data, api_call, assert_error, assert_ok) +from rhodecode.lib import helpers as h from rhodecode.lib.utils2 import safe_unicode pytestmark = pytest.mark.backends("git", "hg") @@ -46,10 +46,10 @@ class TestGetPullRequest(object): assert response.status == '200 OK' url_obj = urlobject.URLObject( - url( + h.route_url( 'pullrequest_show', repo_name=pull_request.target_repo.repo_name, - pull_request_id=pull_request.pull_request_id, qualified=True)) + pull_request_id=pull_request.pull_request_id)) pr_url = safe_unicode( url_obj.with_netloc(http_host_only_stub)) diff --git a/rhodecode/apps/repository/__init__.py b/rhodecode/apps/repository/__init__.py --- a/rhodecode/apps/repository/__init__.py +++ b/rhodecode/apps/repository/__init__.py @@ -217,7 +217,7 @@ def includeme(config): # Pull Requests config.add_route( name='pullrequest_show', - pattern='/{repo_name:.*?[^/]}/pull-request/{pull_request_id}', + pattern='/{repo_name:.*?[^/]}/pull-request/{pull_request_id:\d+}', repo_route=True) config.add_route( @@ -230,6 +230,51 @@ def includeme(config): pattern='/{repo_name:.*?[^/]}/pull-request-data', repo_route=True, repo_accepted_types=['hg', 'git']) + config.add_route( + name='pullrequest_repo_refs', + pattern='/{repo_name:.*?[^/]}/pull-request/refs/{target_repo_name:.*?[^/]}', + repo_route=True) + + config.add_route( + name='pullrequest_repo_destinations', + pattern='/{repo_name:.*?[^/]}/pull-request/repo-destinations', + repo_route=True) + + config.add_route( + name='pullrequest_new', + pattern='/{repo_name:.*?[^/]}/pull-request/new', + repo_route=True, repo_accepted_types=['hg', 'git']) + + config.add_route( + name='pullrequest_create', + pattern='/{repo_name:.*?[^/]}/pull-request/create', + repo_route=True, repo_accepted_types=['hg', 'git']) + + config.add_route( + name='pullrequest_update', + pattern='/{repo_name:.*?[^/]}/pull-request/{pull_request_id:\d+}/update', + repo_route=True) + + config.add_route( + name='pullrequest_merge', + pattern='/{repo_name:.*?[^/]}/pull-request/{pull_request_id:\d+}/merge', + repo_route=True) + + config.add_route( + name='pullrequest_delete', + pattern='/{repo_name:.*?[^/]}/pull-request/{pull_request_id:\d+}/delete', + repo_route=True) + + config.add_route( + name='pullrequest_comment_create', + pattern='/{repo_name:.*?[^/]}/pull-request/{pull_request_id:\d+}/comment', + repo_route=True) + + config.add_route( + name='pullrequest_comment_delete', + pattern='/{repo_name:.*?[^/]}/pull-request/{pull_request_id:\d+}/comment/{comment_id}/delete', + repo_route=True, repo_accepted_types=['hg', 'git']) + # Settings config.add_route( name='edit_repo', diff --git a/rhodecode/tests/functional/test_pullrequests.py b/rhodecode/apps/repository/tests/test_repo_pullrequests.py rename from rhodecode/tests/functional/test_pullrequests.py rename to rhodecode/apps/repository/tests/test_repo_pullrequests.py --- a/rhodecode/tests/functional/test_pullrequests.py +++ b/rhodecode/apps/repository/tests/test_repo_pullrequests.py @@ -17,12 +17,11 @@ # This program is dual-licensed. If you wish to learn more about the # RhodeCode Enterprise Edition, including its added features, Support services, # and proprietary license terms, please see https://rhodecode.com/licenses/ - import mock import pytest -from webob.exc import HTTPNotFound import rhodecode +from rhodecode.lib.vcs.backends.base import MergeResponse, MergeFailureReason from rhodecode.lib.vcs.nodes import FileNode from rhodecode.lib import helpers as h from rhodecode.model.changeset_status import ChangesetStatusModel @@ -32,7 +31,7 @@ from rhodecode.model.meta import Session from rhodecode.model.pull_request import PullRequestModel from rhodecode.model.user import UserModel from rhodecode.tests import ( - assert_session_flash, url, TEST_USER_ADMIN_LOGIN, TEST_USER_REGULAR_LOGIN) + assert_session_flash, TEST_USER_ADMIN_LOGIN, TEST_USER_REGULAR_LOGIN) from rhodecode.tests.utils import AssertResponse @@ -40,8 +39,20 @@ def route_path(name, params=None, **kwar import urllib base_url = { - 'repo_changelog':'/{repo_name}/changelog', - 'repo_changelog_file':'/{repo_name}/changelog/{commit_id}/{f_path}', + 'repo_changelog': '/{repo_name}/changelog', + 'repo_changelog_file': '/{repo_name}/changelog/{commit_id}/{f_path}', + 'pullrequest_show': '/{repo_name}/pull-request/{pull_request_id}', + 'pullrequest_show_all': '/{repo_name}/pull-request', + 'pullrequest_show_all_data': '/{repo_name}/pull-request-data', + 'pullrequest_repo_refs': '/{repo_name}/pull-request/refs/{target_repo_name:.*?[^/]}', + 'pullrequest_repo_destinations': '/{repo_name}/pull-request/repo-destinations', + 'pullrequest_new': '/{repo_name}/pull-request/new', + 'pullrequest_create': '/{repo_name}/pull-request/create', + 'pullrequest_update': '/{repo_name}/pull-request/{pull_request_id}/update', + 'pullrequest_merge': '/{repo_name}/pull-request/{pull_request_id}/merge', + 'pullrequest_delete': '/{repo_name}/pull-request/{pull_request_id}/delete', + 'pullrequest_comment_create': '/{repo_name}/pull-request/{pull_request_id}/comment', + 'pullrequest_comment_delete': '/{repo_name}/pull-request/{pull_request_id}/comment/{comment_id}/delete', }[name].format(**kwargs) if params: @@ -51,26 +62,26 @@ def route_path(name, params=None, **kwar @pytest.mark.usefixtures('app', 'autologin_user') @pytest.mark.backends("git", "hg") -class TestPullrequestsController(object): +class TestPullrequestsView(object): def test_index(self, backend): - self.app.get(url( - controller='pullrequests', action='index', + self.app.get(route_path( + 'pullrequest_new', repo_name=backend.repo_name)) def test_option_menu_create_pull_request_exists(self, backend): repo_name = backend.repo_name response = self.app.get(h.route_path('repo_summary', repo_name=repo_name)) - create_pr_link = 'Create Pull Request' % url( - 'pullrequest', repo_name=repo_name) + create_pr_link = 'Create Pull Request' % route_path( + 'pullrequest_new', repo_name=repo_name) response.mustcontain(create_pr_link) def test_create_pr_form_with_raw_commit_id(self, backend): repo = backend.repo self.app.get( - url(controller='pullrequests', action='index', + route_path('pullrequest_new', repo_name=repo.repo_name, commit=repo.get_commit().raw_id), status=200) @@ -80,10 +91,10 @@ class TestPullrequestsController(object) pull_request = pr_util.create_pull_request( mergeable=pr_merge_enabled, enable_notifications=False) - response = self.app.get(url( - controller='pullrequests', action='show', + response = self.app.get(route_path( + 'pullrequest_show', repo_name=pull_request.target_repo.scm_instance().name, - pull_request_id=str(pull_request.pull_request_id))) + pull_request_id=pull_request.pull_request_id)) for commit_id in pull_request.revisions: response.mustcontain(commit_id) @@ -111,10 +122,10 @@ class TestPullrequestsController(object) pull_request = pr_util.create_pull_request( author=TEST_USER_REGULAR_LOGIN) - response = self.app.get(url( - controller='pullrequests', action='show', + response = self.app.get(route_path( + 'pullrequest_show', repo_name=pull_request.target_repo.scm_instance().name, - pull_request_id=str(pull_request.pull_request_id))) + pull_request_id=pull_request.pull_request_id)) response.mustcontain('Server-side pull request merging is disabled.') @@ -126,10 +137,10 @@ class TestPullrequestsController(object) pull_request.target_repo, UserModel().get_by_username(TEST_USER_REGULAR_LOGIN), 'repository.write') - response = self.app.get(url( - controller='pullrequests', action='show', + response = self.app.get(route_path( + 'pullrequest_show', repo_name=pull_request.target_repo.scm_instance().name, - pull_request_id=str(pull_request.pull_request_id))) + pull_request_id=pull_request.pull_request_id)) response.mustcontain('Server-side pull request merging is disabled.') @@ -144,10 +155,10 @@ class TestPullrequestsController(object) Session().add(pull_request) Session().commit() - response = self.app.get(url( - controller='pullrequests', action='show', + response = self.app.get(route_path( + 'pullrequest_show', repo_name=pull_request.target_repo.scm_instance().name, - pull_request_id=str(pull_request.pull_request_id))) + pull_request_id=pull_request.pull_request_id)) for commit_id in pull_request.revisions: response.mustcontain(commit_id) @@ -158,22 +169,21 @@ class TestPullrequestsController(object) Session().add(pull_request) Session().commit() - self.app.get(url( - controller='pullrequests', action='show', + self.app.get(route_path( + 'pullrequest_show', repo_name=pull_request.target_repo.scm_instance().name, - pull_request_id=str(pull_request.pull_request_id))) + pull_request_id=pull_request.pull_request_id)) def test_edit_title_description(self, pr_util, csrf_token): pull_request = pr_util.create_pull_request() pull_request_id = pull_request.pull_request_id response = self.app.post( - url(controller='pullrequests', action='update', + route_path('pullrequest_update', repo_name=pull_request.target_repo.repo_name, - pull_request_id=str(pull_request_id)), + pull_request_id=pull_request_id), params={ 'edit_pull_request': 'true', - '_method': 'put', 'title': 'New title', 'description': 'New description', 'csrf_token': csrf_token}) @@ -192,12 +202,11 @@ class TestPullrequestsController(object) pr_util.close() response = self.app.post( - url(controller='pullrequests', action='update', + route_path('pullrequest_update', repo_name=pull_request.target_repo.repo_name, - pull_request_id=str(pull_request_id)), + pull_request_id=pull_request_id), params={ 'edit_pull_request': 'true', - '_method': 'put', 'title': 'New title', 'description': 'New description', 'csrf_token': csrf_token}) @@ -217,10 +226,10 @@ class TestPullrequestsController(object) pull_request_id = pull_request.pull_request_id response = self.app.post( - url(controller='pullrequests', action='update', + route_path('pullrequest_update', repo_name=pull_request.target_repo.repo_name, - pull_request_id=str(pull_request_id)), - params={'update_commits': 'true', '_method': 'put', + pull_request_id=pull_request_id), + params={'update_commits': 'true', 'csrf_token': csrf_token}) expected_msg = PullRequestModel.UPDATE_STATUS_MESSAGES[ @@ -236,10 +245,10 @@ class TestPullrequestsController(object) Session().commit() pull_request_id = pull_request.pull_request_id - pull_request_url = url( - controller='pullrequests', action='show', + pull_request_url = route_path( + 'pullrequest_show', repo_name=pull_request.target_repo.repo_name, - pull_request_id=str(pull_request_id)) + pull_request_id=pull_request_id) response = self.app.get(pull_request_url) @@ -258,10 +267,9 @@ class TestPullrequestsController(object) repo = pull_request.target_repo.repo_id self.app.post( - url(controller='pullrequests', - action='comment', + route_path('pullrequest_comment_create', repo_name=pull_request.target_repo.scm_instance().name, - pull_request_id=str(pull_request_id)), + pull_request_id=pull_request_id), params={ 'close_pull_request': '1', 'text': 'Closing a PR', @@ -298,10 +306,9 @@ class TestPullrequestsController(object) repo = pull_request.target_repo.repo_id self.app.post( - url(controller='pullrequests', - action='comment', + route_path('pullrequest_comment_create', repo_name=pull_request.target_repo.scm_instance().name, - pull_request_id=str(pull_request_id)), + pull_request_id=pull_request_id), params={ 'close_pull_request': '1', 'csrf_token': csrf_token}, @@ -326,10 +333,9 @@ class TestPullrequestsController(object) pull_request_id = pull_request.pull_request_id response = self.app.post( - url(controller='pullrequests', - action='comment', + route_path('pullrequest_comment_create', repo_name=pull_request.target_repo.scm_instance().name, - pull_request_id=str(pull_request.pull_request_id)), + pull_request_id=pull_request.pull_request_id), params={ 'close_pull_request': 'true', 'csrf_token': csrf_token}, @@ -356,11 +362,7 @@ class TestPullrequestsController(object) source = backend.create_repo(heads=['change2']) response = self.app.post( - url( - controller='pullrequests', - action='create', - repo_name=source.repo_name - ), + route_path('pullrequest_create', repo_name=source.repo_name), [ ('source_repo', source.repo_name), ('source_ref', 'branch:default:' + commit_ids['change2']), @@ -417,11 +419,7 @@ class TestPullrequestsController(object) source = backend.create_repo(heads=['change']) response = self.app.post( - url( - controller='pullrequests', - action='create', - repo_name=source.repo_name - ), + route_path('pullrequest_create', repo_name=source.repo_name), [ ('source_repo', source.repo_name), ('source_ref', 'branch:default:' + commit_ids['change']), @@ -485,11 +483,7 @@ class TestPullrequestsController(object) source = backend.create_repo(heads=['change']) response = self.app.post( - url( - controller='pullrequests', - action='create', - repo_name=source.repo_name - ), + route_path('pullrequest_create', repo_name=source.repo_name), [ ('source_repo', source.repo_name), ('source_ref', 'branch:default:' + commit_ids['change']), @@ -542,10 +536,9 @@ class TestPullrequestsController(object) repo_name = pull_request.target_repo.scm_instance().name, response = self.app.post( - url(controller='pullrequests', - action='merge', + route_path('pullrequest_merge', repo_name=str(repo_name[0]), - pull_request_id=str(pull_request_id)), + pull_request_id=pull_request_id), params={'csrf_token': csrf_token}).follow() pull_request = PullRequest.get(pull_request_id) @@ -584,10 +577,9 @@ class TestPullrequestsController(object) pull_request = PullRequest.get(pull_request_id) response = self.app.post( - url(controller='pullrequests', - action='merge', + route_path('pullrequest_merge', repo_name=pull_request.target_repo.scm_instance().name, - pull_request_id=str(pull_request.pull_request_id)), + pull_request_id=pull_request.pull_request_id), params={'csrf_token': csrf_token}).follow() assert response.status_int == 200 @@ -599,13 +591,12 @@ class TestPullrequestsController(object) def test_merge_pull_request_not_approved(self, pr_util, csrf_token): pull_request = pr_util.create_pull_request(mergeable=True) pull_request_id = pull_request.pull_request_id - repo_name = pull_request.target_repo.scm_instance().name, + repo_name = pull_request.target_repo.scm_instance().name response = self.app.post( - url(controller='pullrequests', - action='merge', - repo_name=str(repo_name[0]), - pull_request_id=str(pull_request_id)), + route_path('pullrequest_merge', + repo_name=repo_name, + pull_request_id=pull_request_id), params={'csrf_token': csrf_token}).follow() assert response.status_int == 200 @@ -614,6 +605,28 @@ class TestPullrequestsController(object) 'Merge is not currently possible because of below failed checks.') response.mustcontain('Pull request reviewer approval is pending.') + def test_merge_pull_request_renders_failure_reason( + self, user_regular, csrf_token, pr_util): + pull_request = pr_util.create_pull_request(mergeable=True, approved=True) + pull_request_id = pull_request.pull_request_id + repo_name = pull_request.target_repo.scm_instance().name + + model_patcher = mock.patch.multiple( + PullRequestModel, + merge=mock.Mock(return_value=MergeResponse( + True, False, 'STUB_COMMIT_ID', MergeFailureReason.PUSH_FAILED)), + merge_status=mock.Mock(return_value=(True, 'WRONG_MESSAGE'))) + + with model_patcher: + response = self.app.post( + route_path('pullrequest_merge', + repo_name=repo_name, + pull_request_id=pull_request_id), + params={'csrf_token': csrf_token}, status=302) + + assert_session_flash(response, PullRequestModel.MERGE_STATUS_MESSAGES[ + MergeFailureReason.PUSH_FAILED]) + def test_update_source_revision(self, backend, csrf_token): commits = [ {'message': 'ancestor'}, @@ -649,10 +662,10 @@ class TestPullrequestsController(object) # update PR self.app.post( - url(controller='pullrequests', action='update', + route_path('pullrequest_update', repo_name=target.repo_name, - pull_request_id=str(pull_request_id)), - params={'update_commits': 'true', '_method': 'put', + pull_request_id=pull_request_id), + params={'update_commits': 'true', 'csrf_token': csrf_token}) # check that we have now both revisions @@ -661,8 +674,8 @@ class TestPullrequestsController(object) commit_ids['change-2'], commit_ids['change']] # TODO: johbo: this should be a test on its own - response = self.app.get(url( - controller='pullrequests', action='index', + response = self.app.get(route_path( + 'pullrequest_new', repo_name=target.repo_name)) assert response.status_int == 200 assert 'Pull request updated to' in response.body @@ -707,10 +720,10 @@ class TestPullrequestsController(object) # update PR self.app.post( - url(controller='pullrequests', action='update', + route_path('pullrequest_update', repo_name=target.repo_name, - pull_request_id=str(pull_request_id)), - params={'update_commits': 'true', '_method': 'put', + pull_request_id=pull_request_id), + params={'update_commits': 'true', 'csrf_token': csrf_token}, status=200) @@ -722,8 +735,8 @@ class TestPullrequestsController(object) commit_id=commit_ids['ancestor-new']) # TODO: johbo: This should be a test on its own - response = self.app.get(url( - controller='pullrequests', action='index', + response = self.app.get(route_path( + 'pullrequest_new', repo_name=target.repo_name)) assert response.status_int == 200 assert 'Pull request updated to' in response.body @@ -770,10 +783,10 @@ class TestPullrequestsController(object) # update PR self.app.post( - url(controller='pullrequests', action='update', + route_path('pullrequest_update', repo_name=target.repo_name, - pull_request_id=str(pull_request_id)), - params={'update_commits': 'true', '_method': 'put', + pull_request_id=pull_request_id), + params={'update_commits': 'true', 'csrf_token': csrf_token}, status=200) @@ -814,10 +827,10 @@ class TestPullrequestsController(object) vcs = repo.scm_instance() vcs.remove_ref('refs/heads/{}'.format(branch_name)) - response = self.app.get(url( - controller='pullrequests', action='show', + response = self.app.get(route_path( + 'pullrequest_show', repo_name=repo.repo_name, - pull_request_id=str(pull_request.pull_request_id))) + pull_request_id=pull_request.pull_request_id)) assert response.status_int == 200 assert_response = AssertResponse(response) @@ -846,10 +859,10 @@ class TestPullrequestsController(object) else: vcs.strip(pr_util.commit_ids['new-feature']) - response = self.app.get(url( - controller='pullrequests', action='show', + response = self.app.get(route_path( + 'pullrequest_show', repo_name=pr_util.target_repository.repo_name, - pull_request_id=str(pull_request.pull_request_id))) + pull_request_id=pull_request.pull_request_id)) assert response.status_int == 200 assert_response = AssertResponse(response) @@ -882,20 +895,20 @@ class TestPullrequestsController(object) vcs.strip(pr_util.commit_ids['new-feature']) response = self.app.post( - url(controller='pullrequests', action='update', + route_path('pullrequest_update', repo_name=pull_request.target_repo.repo_name, - pull_request_id=str(pull_request.pull_request_id)), - params={'update_commits': 'true', '_method': 'put', + pull_request_id=pull_request.pull_request_id), + params={'update_commits': 'true', 'csrf_token': csrf_token}) assert response.status_int == 200 assert response.body == 'true' # Make sure that after update, it won't raise 500 errors - response = self.app.get(url( - controller='pullrequests', action='show', + response = self.app.get(route_path( + 'pullrequest_show', repo_name=pr_util.target_repository.repo_name, - pull_request_id=str(pull_request.pull_request_id))) + pull_request_id=pull_request.pull_request_id)) assert response.status_int == 200 assert_response = AssertResponse(response) @@ -910,10 +923,10 @@ class TestPullrequestsController(object) Session().add(pull_request) Session().commit() - response = self.app.get(url( - controller='pullrequests', action='show', + response = self.app.get(route_path( + 'pullrequest_show', repo_name=pull_request.target_repo.scm_instance().name, - pull_request_id=str(pull_request.pull_request_id))) + pull_request_id=pull_request.pull_request_id)) assert response.status_int == 200 assert_response = AssertResponse(response) @@ -944,10 +957,10 @@ class TestPullrequestsController(object) Session().add(pull_request) Session().commit() - response = self.app.get(url( - controller='pullrequests', action='show', + response = self.app.get(route_path( + 'pullrequest_show', repo_name=pull_request.target_repo.scm_instance().name, - pull_request_id=str(pull_request.pull_request_id))) + pull_request_id=pull_request.pull_request_id)) assert response.status_int == 200 assert_response = AssertResponse(response) @@ -966,10 +979,10 @@ class TestPullrequestsController(object) Session().add(pull_request) Session().commit() - response = self.app.get(url( - controller='pullrequests', action='show', + response = self.app.get(route_path( + 'pullrequest_show', repo_name=pull_request.target_repo.scm_instance().name, - pull_request_id=str(pull_request.pull_request_id))) + pull_request_id=pull_request.pull_request_id)) assert response.status_int == 200 assert_response = AssertResponse(response) @@ -996,10 +1009,10 @@ class TestPullrequestsController(object) shadow_url = '{host}/{repo}/pull-request/{pr_id}/repository'.format( host=http_host_only_stub, repo=target_repo.name, pr_id=pr_id) - response = self.app.get(url( - controller='pullrequests', action='show', + response = self.app.get(route_path( + 'pullrequest_show', repo_name=target_repo.name, - pull_request_id=str(pr_id))) + pull_request_id=pr_id)) assertr = AssertResponse(response) if mergeable: @@ -1019,10 +1032,10 @@ class TestPullrequestsControllerDelete(o pull_request = pr_util.create_pull_request( author=user_admin.username, enable_notifications=False) - response = self.app.get(url( - controller='pullrequests', action='show', + response = self.app.get(route_path( + 'pullrequest_show', repo_name=pull_request.target_repo.scm_instance().name, - pull_request_id=str(pull_request.pull_request_id))) + pull_request_id=pull_request.pull_request_id)) response.mustcontain('id="delete_pullrequest"') response.mustcontain('Confirm to delete this pull request') @@ -1032,10 +1045,10 @@ class TestPullrequestsControllerDelete(o pull_request = pr_util.create_pull_request( author=user_regular.username, enable_notifications=False) - response = self.app.get(url( - controller='pullrequests', action='show', + response = self.app.get(route_path( + 'pullrequest_show', repo_name=pull_request.target_repo.scm_instance().name, - pull_request_id=str(pull_request.pull_request_id))) + pull_request_id=pull_request.pull_request_id)) response.mustcontain('id="delete_pullrequest"') response.mustcontain('Confirm to delete this pull request') @@ -1045,10 +1058,10 @@ class TestPullrequestsControllerDelete(o pull_request = pr_util.create_pull_request( author=user_admin.username, enable_notifications=False) - response = self.app.get(url( - controller='pullrequests', action='show', + response = self.app.get(route_path( + 'pullrequest_show', repo_name=pull_request.target_repo.scm_instance().name, - pull_request_id=str(pull_request.pull_request_id))) + pull_request_id=pull_request.pull_request_id)) response.mustcontain(no=['id="delete_pullrequest"']) response.mustcontain(no=['Confirm to delete this pull request']) @@ -1063,10 +1076,10 @@ class TestPullrequestsControllerDelete(o pull_request.target_repo, user_regular, 'repository.write') - response = self.app.get(url( - controller='pullrequests', action='show', + response = self.app.get(route_path( + 'pullrequest_show', repo_name=pull_request.target_repo.scm_instance().name, - pull_request_id=str(pull_request.pull_request_id))) + pull_request_id=pull_request.pull_request_id)) response.mustcontain('id="open_edit_pullrequest"') response.mustcontain('id="delete_pullrequest"') @@ -1078,9 +1091,10 @@ class TestPullrequestsControllerDelete(o pull_request = pr_util.create_pull_request( author=user_admin.username, enable_notifications=False) - self.app.get(url( - controller='pullrequests', action='delete_comment', + self.app.get(route_path( + 'pullrequest_comment_delete', repo_name=pull_request.target_repo.scm_instance().name, + pull_request_id=pull_request.pull_request_id, comment_id=1024404), status=404) @@ -1090,17 +1104,9 @@ def assert_pull_request_status(pull_requ assert status == expected_status -@pytest.mark.parametrize('action', ['index', 'create']) +@pytest.mark.parametrize('route', ['pullrequest_new', 'pullrequest_create']) @pytest.mark.usefixtures("autologin_user") -def test_redirects_to_repo_summary_for_svn_repositories(backend_svn, app, action): - response = app.get(url( - controller='pullrequests', action=action, - repo_name=backend_svn.repo_name)) - assert response.status_int == 302 +def test_forbidde_to_repo_summary_for_svn_repositories(backend_svn, app, route): + response = app.get( + route_path(route, repo_name=backend_svn.repo_name), status=404) - # Not allowed, redirect to the summary - redirected = response.follow() - summary_url = h.route_path('repo_summary', repo_name=backend_svn.repo_name) - - # URL adds leading slash and path doesn't have it - assert redirected.request.path == summary_url diff --git a/rhodecode/apps/repository/views/repo_pull_requests.py b/rhodecode/apps/repository/views/repo_pull_requests.py --- a/rhodecode/apps/repository/views/repo_pull_requests.py +++ b/rhodecode/apps/repository/views/repo_pull_requests.py @@ -19,23 +19,35 @@ # and proprietary license terms, please see https://rhodecode.com/licenses/ import logging +import collections -import collections -from pyramid.httpexceptions import HTTPFound, HTTPNotFound +import formencode +import peppercorn +from pyramid.httpexceptions import ( + HTTPFound, HTTPNotFound, HTTPForbidden, HTTPBadRequest) from pyramid.view import view_config +from pyramid.renderers import render +from rhodecode import events from rhodecode.apps._base import RepoAppView, DataGridAppView -from rhodecode.lib import helpers as h, diffs, codeblocks + +from rhodecode.lib import helpers as h, diffs, codeblocks, channelstream +from rhodecode.lib.base import vcs_operation_context +from rhodecode.lib.ext_json import json from rhodecode.lib.auth import ( - LoginRequired, HasRepoPermissionAnyDecorator) -from rhodecode.lib.utils2 import str2bool, safe_int, safe_str -from rhodecode.lib.vcs.backends.base import EmptyCommit -from rhodecode.lib.vcs.exceptions import CommitDoesNotExistError, \ - RepositoryRequirementError, NodeDoesNotExistError + LoginRequired, HasRepoPermissionAnyDecorator, NotAnonymous, CSRFRequired) +from rhodecode.lib.utils2 import str2bool, safe_str, safe_unicode +from rhodecode.lib.vcs.backends.base import EmptyCommit, UpdateFailureReason +from rhodecode.lib.vcs.exceptions import (CommitDoesNotExistError, + RepositoryRequirementError, NodeDoesNotExistError, EmptyRepositoryError) +from rhodecode.model.changeset_status import ChangesetStatusModel from rhodecode.model.comment import CommentsModel -from rhodecode.model.db import PullRequest, PullRequestVersion, \ - ChangesetComment, ChangesetStatus +from rhodecode.model.db import (func, or_, PullRequest, PullRequestVersion, + ChangesetComment, ChangesetStatus, Repository) +from rhodecode.model.forms import PullRequestForm +from rhodecode.model.meta import Session from rhodecode.model.pull_request import PullRequestModel, MergeCheck +from rhodecode.model.scm import ScmModel log = logging.getLogger(__name__) @@ -189,7 +201,6 @@ class RepoPullRequestsView(RepoAppView, return data def _get_pr_version(self, pull_request_id, version=None): - pull_request_id = safe_int(pull_request_id) at_version = None if version and version == 'latest': @@ -250,12 +261,12 @@ class RepoPullRequestsView(RepoAppView, @LoginRequired() @HasRepoPermissionAnyDecorator( 'repository.read', 'repository.write', 'repository.admin') - # @view_config( - # route_name='pullrequest_show', request_method='GET', - # renderer='rhodecode:templates/pullrequests/pullrequest_show.mako') + @view_config( + route_name='pullrequest_show', request_method='GET', + renderer='rhodecode:templates/pullrequests/pullrequest_show.mako') def pull_request_show(self): - pull_request_id = safe_int( - self.request.matchdict.get('pull_request_id')) + pull_request_id = self.request.matchdict.get('pull_request_id') + c = self.load_default_context() version = self.request.GET.get('version') @@ -582,3 +593,590 @@ class RepoPullRequestsView(RepoAppView, c.review_versions[_ver_pr] = status[0] return self._get_template_context(c) + + def assure_not_empty_repo(self): + _ = self.request.translate + + try: + self.db_repo.scm_instance().get_commit() + except EmptyRepositoryError: + h.flash(h.literal(_('There are no commits yet')), + category='warning') + raise HTTPFound( + h.route_path('repo_summary', repo_name=self.db_repo.repo_name)) + + @LoginRequired() + @NotAnonymous() + @HasRepoPermissionAnyDecorator( + 'repository.read', 'repository.write', 'repository.admin') + @view_config( + route_name='pullrequest_new', request_method='GET', + renderer='rhodecode:templates/pullrequests/pullrequest.mako') + def pull_request_new(self): + _ = self.request.translate + c = self.load_default_context() + + self.assure_not_empty_repo() + source_repo = self.db_repo + + commit_id = self.request.GET.get('commit') + branch_ref = self.request.GET.get('branch') + bookmark_ref = self.request.GET.get('bookmark') + + try: + source_repo_data = PullRequestModel().generate_repo_data( + source_repo, commit_id=commit_id, + branch=branch_ref, bookmark=bookmark_ref) + except CommitDoesNotExistError as e: + log.exception(e) + h.flash(_('Commit does not exist'), 'error') + raise HTTPFound( + h.route_path('pullrequest_new', repo_name=source_repo.repo_name)) + + default_target_repo = source_repo + + if source_repo.parent: + parent_vcs_obj = source_repo.parent.scm_instance() + if parent_vcs_obj and not parent_vcs_obj.is_empty(): + # change default if we have a parent repo + default_target_repo = source_repo.parent + + target_repo_data = PullRequestModel().generate_repo_data( + default_target_repo) + + selected_source_ref = source_repo_data['refs']['selected_ref'] + + title_source_ref = selected_source_ref.split(':', 2)[1] + c.default_title = PullRequestModel().generate_pullrequest_title( + source=source_repo.repo_name, + source_ref=title_source_ref, + target=default_target_repo.repo_name + ) + + c.default_repo_data = { + 'source_repo_name': source_repo.repo_name, + 'source_refs_json': json.dumps(source_repo_data), + 'target_repo_name': default_target_repo.repo_name, + 'target_refs_json': json.dumps(target_repo_data), + } + c.default_source_ref = selected_source_ref + + return self._get_template_context(c) + + @LoginRequired() + @NotAnonymous() + @HasRepoPermissionAnyDecorator( + 'repository.read', 'repository.write', 'repository.admin') + @view_config( + route_name='pullrequest_repo_refs', request_method='GET', + renderer='json_ext', xhr=True) + def pull_request_repo_refs(self): + target_repo_name = self.request.matchdict['target_repo_name'] + repo = Repository.get_by_repo_name(target_repo_name) + if not repo: + raise HTTPNotFound() + return PullRequestModel().generate_repo_data(repo) + + @LoginRequired() + @NotAnonymous() + @HasRepoPermissionAnyDecorator( + 'repository.read', 'repository.write', 'repository.admin') + @view_config( + route_name='pullrequest_repo_destinations', request_method='GET', + renderer='json_ext', xhr=True) + def pull_request_repo_destinations(self): + _ = self.request.translate + filter_query = self.request.GET.get('query') + + query = Repository.query() \ + .order_by(func.length(Repository.repo_name)) \ + .filter( + or_(Repository.repo_name == self.db_repo.repo_name, + Repository.fork_id == self.db_repo.repo_id)) + + if filter_query: + ilike_expression = u'%{}%'.format(safe_unicode(filter_query)) + query = query.filter( + Repository.repo_name.ilike(ilike_expression)) + + add_parent = False + if self.db_repo.parent: + if filter_query in self.db_repo.parent.repo_name: + parent_vcs_obj = self.db_repo.parent.scm_instance() + if parent_vcs_obj and not parent_vcs_obj.is_empty(): + add_parent = True + + limit = 20 - 1 if add_parent else 20 + all_repos = query.limit(limit).all() + if add_parent: + all_repos += [self.db_repo.parent] + + repos = [] + for obj in ScmModel().get_repos(all_repos): + repos.append({ + 'id': obj['name'], + 'text': obj['name'], + 'type': 'repo', + 'obj': obj['dbrepo'] + }) + + data = { + 'more': False, + 'results': [{ + 'text': _('Repositories'), + 'children': repos + }] if repos else [] + } + return data + + @LoginRequired() + @NotAnonymous() + @HasRepoPermissionAnyDecorator( + 'repository.read', 'repository.write', 'repository.admin') + @CSRFRequired() + @view_config( + route_name='pullrequest_create', request_method='POST', + renderer=None) + def pull_request_create(self): + _ = self.request.translate + self.assure_not_empty_repo() + + controls = peppercorn.parse(self.request.POST.items()) + + try: + _form = PullRequestForm(self.db_repo.repo_id)().to_python(controls) + except formencode.Invalid as errors: + if errors.error_dict.get('revisions'): + msg = 'Revisions: %s' % errors.error_dict['revisions'] + elif errors.error_dict.get('pullrequest_title'): + msg = _('Pull request requires a title with min. 3 chars') + else: + msg = _('Error creating pull request: {}').format(errors) + log.exception(msg) + h.flash(msg, 'error') + + # would rather just go back to form ... + raise HTTPFound( + h.route_path('pullrequest_new', repo_name=self.db_repo_name)) + + source_repo = _form['source_repo'] + source_ref = _form['source_ref'] + target_repo = _form['target_repo'] + target_ref = _form['target_ref'] + commit_ids = _form['revisions'][::-1] + + # find the ancestor for this pr + source_db_repo = Repository.get_by_repo_name(_form['source_repo']) + target_db_repo = Repository.get_by_repo_name(_form['target_repo']) + + source_scm = source_db_repo.scm_instance() + target_scm = target_db_repo.scm_instance() + + source_commit = source_scm.get_commit(source_ref.split(':')[-1]) + target_commit = target_scm.get_commit(target_ref.split(':')[-1]) + + ancestor = source_scm.get_common_ancestor( + source_commit.raw_id, target_commit.raw_id, target_scm) + + target_ref_type, target_ref_name, __ = _form['target_ref'].split(':') + target_ref = ':'.join((target_ref_type, target_ref_name, ancestor)) + + pullrequest_title = _form['pullrequest_title'] + title_source_ref = source_ref.split(':', 2)[1] + if not pullrequest_title: + pullrequest_title = PullRequestModel().generate_pullrequest_title( + source=source_repo, + source_ref=title_source_ref, + target=target_repo + ) + + description = _form['pullrequest_desc'] + + get_default_reviewers_data, validate_default_reviewers = \ + PullRequestModel().get_reviewer_functions() + + # recalculate reviewers logic, to make sure we can validate this + reviewer_rules = get_default_reviewers_data( + self._rhodecode_db_user, source_db_repo, + source_commit, target_db_repo, target_commit) + + given_reviewers = _form['review_members'] + reviewers = validate_default_reviewers(given_reviewers, reviewer_rules) + + try: + pull_request = PullRequestModel().create( + self._rhodecode_user.user_id, source_repo, source_ref, target_repo, + target_ref, commit_ids, reviewers, pullrequest_title, + description, reviewer_rules + ) + Session().commit() + h.flash(_('Successfully opened new pull request'), + category='success') + except Exception as e: + msg = _('Error occurred during creation of this pull request.') + log.exception(msg) + h.flash(msg, category='error') + raise HTTPFound( + h.route_path('pullrequest_new', repo_name=self.db_repo_name)) + + raise HTTPFound( + h.route_path('pullrequest_show', repo_name=target_repo, + pull_request_id=pull_request.pull_request_id)) + + @LoginRequired() + @NotAnonymous() + @HasRepoPermissionAnyDecorator( + 'repository.read', 'repository.write', 'repository.admin') + @CSRFRequired() + @view_config( + route_name='pullrequest_update', request_method='POST', + renderer='json_ext') + def pull_request_update(self): + pull_request_id = self.request.matchdict['pull_request_id'] + pull_request = PullRequest.get_or_404(pull_request_id) + + # only owner or admin can update it + allowed_to_update = PullRequestModel().check_user_update( + pull_request, self._rhodecode_user) + if allowed_to_update: + controls = peppercorn.parse(self.request.POST.items()) + + if 'review_members' in controls: + self._update_reviewers( + pull_request_id, controls['review_members'], + pull_request.reviewer_data) + elif str2bool(self.request.POST.get('update_commits', 'false')): + self._update_commits(pull_request) + elif str2bool(self.request.POST.get('edit_pull_request', 'false')): + self._edit_pull_request(pull_request) + else: + raise HTTPBadRequest() + return True + raise HTTPForbidden() + + def _edit_pull_request(self, pull_request): + _ = self.request.translate + try: + PullRequestModel().edit( + pull_request, self.request.POST.get('title'), + self.request.POST.get('description'), self._rhodecode_user) + except ValueError: + msg = _(u'Cannot update closed pull requests.') + h.flash(msg, category='error') + return + else: + Session().commit() + + msg = _(u'Pull request title & description updated.') + h.flash(msg, category='success') + return + + def _update_commits(self, pull_request): + _ = self.request.translate + resp = PullRequestModel().update_commits(pull_request) + + if resp.executed: + + if resp.target_changed and resp.source_changed: + changed = 'target and source repositories' + elif resp.target_changed and not resp.source_changed: + changed = 'target repository' + elif not resp.target_changed and resp.source_changed: + changed = 'source repository' + else: + changed = 'nothing' + + msg = _( + u'Pull request updated to "{source_commit_id}" with ' + u'{count_added} added, {count_removed} removed commits. ' + u'Source of changes: {change_source}') + msg = msg.format( + source_commit_id=pull_request.source_ref_parts.commit_id, + count_added=len(resp.changes.added), + count_removed=len(resp.changes.removed), + change_source=changed) + h.flash(msg, category='success') + + channel = '/repo${}$/pr/{}'.format( + pull_request.target_repo.repo_name, + pull_request.pull_request_id) + message = msg + ( + ' - ' + '{}'.format(_('Reload page'))) + channelstream.post_message( + channel, message, self._rhodecode_user.username, + registry=self.request.registry) + else: + msg = PullRequestModel.UPDATE_STATUS_MESSAGES[resp.reason] + warning_reasons = [ + UpdateFailureReason.NO_CHANGE, + UpdateFailureReason.WRONG_REF_TYPE, + ] + category = 'warning' if resp.reason in warning_reasons else 'error' + h.flash(msg, category=category) + + @LoginRequired() + @NotAnonymous() + @HasRepoPermissionAnyDecorator( + 'repository.read', 'repository.write', 'repository.admin') + @CSRFRequired() + @view_config( + route_name='pullrequest_merge', request_method='POST', + renderer='json_ext') + def pull_request_merge(self): + """ + Merge will perform a server-side merge of the specified + pull request, if the pull request is approved and mergeable. + After successful merging, the pull request is automatically + closed, with a relevant comment. + """ + pull_request_id = self.request.matchdict['pull_request_id'] + pull_request = PullRequest.get_or_404(pull_request_id) + + check = MergeCheck.validate(pull_request, self._rhodecode_db_user) + merge_possible = not check.failed + + for err_type, error_msg in check.errors: + h.flash(error_msg, category=err_type) + + if merge_possible: + log.debug("Pre-conditions checked, trying to merge.") + extras = vcs_operation_context( + self.request.environ, repo_name=pull_request.target_repo.repo_name, + username=self._rhodecode_db_user.username, action='push', + scm=pull_request.target_repo.repo_type) + self._merge_pull_request( + pull_request, self._rhodecode_db_user, extras) + else: + log.debug("Pre-conditions failed, NOT merging.") + + raise HTTPFound( + h.route_path('pullrequest_show', + repo_name=pull_request.target_repo.repo_name, + pull_request_id=pull_request.pull_request_id)) + + def _merge_pull_request(self, pull_request, user, extras): + _ = self.request.translate + merge_resp = PullRequestModel().merge(pull_request, user, extras=extras) + + if merge_resp.executed: + log.debug("The merge was successful, closing the pull request.") + PullRequestModel().close_pull_request( + pull_request.pull_request_id, user) + Session().commit() + msg = _('Pull request was successfully merged and closed.') + h.flash(msg, category='success') + else: + log.debug( + "The merge was not successful. Merge response: %s", + merge_resp) + msg = PullRequestModel().merge_status_message( + merge_resp.failure_reason) + h.flash(msg, category='error') + + def _update_reviewers(self, pull_request_id, review_members, reviewer_rules): + _ = self.request.translate + get_default_reviewers_data, validate_default_reviewers = \ + PullRequestModel().get_reviewer_functions() + + try: + reviewers = validate_default_reviewers(review_members, reviewer_rules) + except ValueError as e: + log.error('Reviewers Validation: {}'.format(e)) + h.flash(e, category='error') + return + + PullRequestModel().update_reviewers( + pull_request_id, reviewers, self._rhodecode_user) + h.flash(_('Pull request reviewers updated.'), category='success') + Session().commit() + + @LoginRequired() + @NotAnonymous() + @HasRepoPermissionAnyDecorator( + 'repository.read', 'repository.write', 'repository.admin') + @CSRFRequired() + @view_config( + route_name='pullrequest_delete', request_method='POST', + renderer='json_ext') + def pull_request_delete(self): + _ = self.request.translate + + pull_request_id = self.request.matchdict['pull_request_id'] + pull_request = PullRequest.get_or_404(pull_request_id) + + pr_closed = pull_request.is_closed() + allowed_to_delete = PullRequestModel().check_user_delete( + pull_request, self._rhodecode_user) and not pr_closed + + # only owner can delete it ! + if allowed_to_delete: + PullRequestModel().delete(pull_request, self._rhodecode_user) + Session().commit() + h.flash(_('Successfully deleted pull request'), + category='success') + raise HTTPFound(h.route_path('my_account_pullrequests')) + + log.warning('user %s tried to delete pull request without access', + self._rhodecode_user) + raise HTTPNotFound() + + @LoginRequired() + @NotAnonymous() + @HasRepoPermissionAnyDecorator( + 'repository.read', 'repository.write', 'repository.admin') + @CSRFRequired() + @view_config( + route_name='pullrequest_comment_create', request_method='POST', + renderer='json_ext') + def pull_request_comment_create(self): + _ = self.request.translate + pull_request_id = self.request.matchdict['pull_request_id'] + pull_request = PullRequest.get_or_404(pull_request_id) + if pull_request.is_closed(): + log.debug('comment: forbidden because pull request is closed') + raise HTTPForbidden() + + c = self.load_default_context() + + status = self.request.POST.get('changeset_status', None) + text = self.request.POST.get('text') + comment_type = self.request.POST.get('comment_type') + resolves_comment_id = self.request.POST.get('resolves_comment_id', None) + close_pull_request = self.request.POST.get('close_pull_request') + + # the logic here should work like following, if we submit close + # pr comment, use `close_pull_request_with_comment` function + # else handle regular comment logic + + if close_pull_request: + # only owner or admin or person with write permissions + allowed_to_close = PullRequestModel().check_user_update( + pull_request, self._rhodecode_user) + if not allowed_to_close: + log.debug('comment: forbidden because not allowed to close ' + 'pull request %s', pull_request_id) + raise HTTPForbidden() + comment, status = PullRequestModel().close_pull_request_with_comment( + pull_request, self._rhodecode_user, self.db_repo, message=text) + Session().flush() + events.trigger( + events.PullRequestCommentEvent(pull_request, comment)) + + else: + # regular comment case, could be inline, or one with status. + # for that one we check also permissions + + allowed_to_change_status = PullRequestModel().check_user_change_status( + pull_request, self._rhodecode_user) + + if status and allowed_to_change_status: + message = (_('Status change %(transition_icon)s %(status)s') + % {'transition_icon': '>', + 'status': ChangesetStatus.get_status_lbl(status)}) + text = text or message + + comment = CommentsModel().create( + text=text, + repo=self.db_repo.repo_id, + user=self._rhodecode_user.user_id, + pull_request=pull_request_id, + f_path=self.request.POST.get('f_path'), + line_no=self.request.POST.get('line'), + status_change=(ChangesetStatus.get_status_lbl(status) + if status and allowed_to_change_status else None), + status_change_type=(status + if status and allowed_to_change_status else None), + comment_type=comment_type, + resolves_comment_id=resolves_comment_id + ) + + if allowed_to_change_status: + # calculate old status before we change it + old_calculated_status = pull_request.calculated_review_status() + + # get status if set ! + if status: + ChangesetStatusModel().set_status( + self.db_repo.repo_id, + status, + self._rhodecode_user.user_id, + comment, + pull_request=pull_request_id + ) + + Session().flush() + events.trigger( + events.PullRequestCommentEvent(pull_request, comment)) + + # we now calculate the status of pull request, and based on that + # calculation we set the commits status + calculated_status = pull_request.calculated_review_status() + if old_calculated_status != calculated_status: + PullRequestModel()._trigger_pull_request_hook( + pull_request, self._rhodecode_user, 'review_status_change') + + Session().commit() + + data = { + 'target_id': h.safeid(h.safe_unicode( + self.request.POST.get('f_path'))), + } + if comment: + c.co = comment + rendered_comment = render( + 'rhodecode:templates/changeset/changeset_comment_block.mako', + self._get_template_context(c), self.request) + + data.update(comment.get_dict()) + data.update({'rendered_text': rendered_comment}) + + return data + + @LoginRequired() + @NotAnonymous() + @HasRepoPermissionAnyDecorator( + 'repository.read', 'repository.write', 'repository.admin') + @CSRFRequired() + @view_config( + route_name='pullrequest_comment_delete', request_method='POST', + renderer='json_ext') + def pull_request_comment_delete(self): + commit_id = self.request.matchdict['commit_id'] + comment_id = self.request.matchdict['comment_id'] + pull_request_id = self.request.matchdict['pull_request_id'] + + pull_request = PullRequest.get_or_404(pull_request_id) + if pull_request.is_closed(): + log.debug('comment: forbidden because pull request is closed') + raise HTTPForbidden() + + comment = ChangesetComment.get_or_404(comment_id) + if not comment: + log.debug('Comment with id:%s not found, skipping', comment_id) + # comment already deleted in another call probably + return True + + if comment.pull_request.is_closed(): + # don't allow deleting comments on closed pull request + raise HTTPForbidden() + + is_repo_admin = h.HasRepoPermissionAny('repository.admin')(self.db_repo_name) + super_admin = h.HasPermissionAny('hg.admin')() + comment_owner = comment.author.user_id == self._rhodecode_user.user_id + is_repo_comment = comment.repo.repo_name == self.db_repo_name + comment_repo_admin = is_repo_admin and is_repo_comment + + if super_admin or comment_owner or comment_repo_admin: + old_calculated_status = comment.pull_request.calculated_review_status() + CommentsModel().delete(comment=comment, user=self._rhodecode_user) + Session().commit() + calculated_status = comment.pull_request.calculated_review_status() + if old_calculated_status != calculated_status: + PullRequestModel()._trigger_pull_request_hook( + comment.pull_request, self._rhodecode_user, 'review_status_change') + return True + else: + log.warning('No permissions for user %s to delete comment_id: %s', + self._rhodecode_db_user, comment_id) + raise HTTPNotFound() diff --git a/rhodecode/config/routing.py b/rhodecode/config/routing.py --- a/rhodecode/config/routing.py +++ b/rhodecode/config/routing.py @@ -503,73 +503,6 @@ def make_map(config): requirements=URL_NAME_REQUIREMENTS) - rmap.connect('pullrequest_home', - '/{repo_name}/pull-request/new', controller='pullrequests', - action='index', conditions={'function': check_repo, - 'method': ['GET']}, - requirements=URL_NAME_REQUIREMENTS, jsroute=True) - - rmap.connect('pullrequest', - '/{repo_name}/pull-request/new', controller='pullrequests', - action='create', conditions={'function': check_repo, - 'method': ['POST']}, - requirements=URL_NAME_REQUIREMENTS, jsroute=True) - - rmap.connect('pullrequest_repo_refs', - '/{repo_name}/pull-request/refs/{target_repo_name:.*?[^/]}', - controller='pullrequests', - action='get_repo_refs', - conditions={'function': check_repo, 'method': ['GET']}, - requirements=URL_NAME_REQUIREMENTS, jsroute=True) - - rmap.connect('pullrequest_repo_destinations', - '/{repo_name}/pull-request/repo-destinations', - controller='pullrequests', - action='get_repo_destinations', - conditions={'function': check_repo, 'method': ['GET']}, - requirements=URL_NAME_REQUIREMENTS, jsroute=True) - - rmap.connect('pullrequest_show', - '/{repo_name}/pull-request/{pull_request_id}', - controller='pullrequests', - action='show', conditions={'function': check_repo, - 'method': ['GET']}, - requirements=URL_NAME_REQUIREMENTS, jsroute=True) - - rmap.connect('pullrequest_update', - '/{repo_name}/pull-request/{pull_request_id}', - controller='pullrequests', - action='update', conditions={'function': check_repo, - 'method': ['PUT']}, - requirements=URL_NAME_REQUIREMENTS, jsroute=True) - - rmap.connect('pullrequest_merge', - '/{repo_name}/pull-request/{pull_request_id}', - controller='pullrequests', - action='merge', conditions={'function': check_repo, - 'method': ['POST']}, - requirements=URL_NAME_REQUIREMENTS) - - rmap.connect('pullrequest_delete', - '/{repo_name}/pull-request/{pull_request_id}', - controller='pullrequests', - action='delete', conditions={'function': check_repo, - 'method': ['DELETE']}, - requirements=URL_NAME_REQUIREMENTS) - - rmap.connect('pullrequest_comment', - '/{repo_name}/pull-request-comment/{pull_request_id}', - controller='pullrequests', - action='comment', conditions={'function': check_repo, - 'method': ['POST']}, - requirements=URL_NAME_REQUIREMENTS, jsroute=True) - - rmap.connect('pullrequest_comment_delete', - '/{repo_name}/pull-request-comment/{comment_id}/delete', - controller='pullrequests', action='delete_comment', - conditions={'function': check_repo, 'method': ['DELETE']}, - requirements=URL_NAME_REQUIREMENTS, jsroute=True) - rmap.connect('repo_fork_create_home', '/{repo_name}/fork', controller='forks', action='fork_create', conditions={'function': check_repo, 'method': ['POST']}, diff --git a/rhodecode/controllers/pullrequests.py b/rhodecode/controllers/pullrequests.py deleted file mode 100644 --- a/rhodecode/controllers/pullrequests.py +++ /dev/null @@ -1,1018 +0,0 @@ -# -*- coding: utf-8 -*- - -# Copyright (C) 2012-2017 RhodeCode GmbH -# -# This program is free software: you can redistribute it and/or modify -# it under the terms of the GNU Affero General Public License, version 3 -# (only), as published by the Free Software Foundation. -# -# This program is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU General Public License for more details. -# -# You should have received a copy of the GNU Affero General Public License -# along with this program. If not, see . -# -# This program is dual-licensed. If you wish to learn more about the -# RhodeCode Enterprise Edition, including its added features, Support services, -# and proprietary license terms, please see https://rhodecode.com/licenses/ - -""" -pull requests controller for rhodecode for initializing pull requests -""" -import peppercorn -import formencode -import logging -import collections - -from webob.exc import HTTPNotFound, HTTPForbidden, HTTPBadRequest -from pylons import request, tmpl_context as c, url -from pylons.controllers.util import redirect -from pylons.i18n.translation import _ -from pyramid.threadlocal import get_current_registry -from pyramid.httpexceptions import HTTPFound -from sqlalchemy.sql import func -from sqlalchemy.sql.expression import or_ - -from rhodecode import events -from rhodecode.lib import auth, diffs, helpers as h, codeblocks -from rhodecode.lib.ext_json import json -from rhodecode.lib.base import ( - BaseRepoController, render, vcs_operation_context) -from rhodecode.lib.auth import ( - LoginRequired, HasRepoPermissionAnyDecorator, NotAnonymous, - HasAcceptedRepoType, XHRRequired) -from rhodecode.lib.channelstream import channelstream_request -from rhodecode.lib.utils import jsonify -from rhodecode.lib.utils2 import ( - safe_int, safe_str, str2bool, safe_unicode) -from rhodecode.lib.vcs.backends.base import ( - EmptyCommit, UpdateFailureReason, EmptyRepository) -from rhodecode.lib.vcs.exceptions import ( - EmptyRepositoryError, CommitDoesNotExistError, RepositoryRequirementError, - NodeDoesNotExistError) - -from rhodecode.model.changeset_status import ChangesetStatusModel -from rhodecode.model.comment import CommentsModel -from rhodecode.model.db import (PullRequest, ChangesetStatus, ChangesetComment, - Repository, PullRequestVersion) -from rhodecode.model.forms import PullRequestForm -from rhodecode.model.meta import Session -from rhodecode.model.pull_request import PullRequestModel, MergeCheck - -log = logging.getLogger(__name__) - - -class PullrequestsController(BaseRepoController): - - def __before__(self): - super(PullrequestsController, self).__before__() - c.REVIEW_STATUS_APPROVED = ChangesetStatus.STATUS_APPROVED - c.REVIEW_STATUS_REJECTED = ChangesetStatus.STATUS_REJECTED - - @LoginRequired() - @NotAnonymous() - @HasRepoPermissionAnyDecorator('repository.read', 'repository.write', - 'repository.admin') - @HasAcceptedRepoType('git', 'hg') - def index(self): - source_repo = c.rhodecode_db_repo - - try: - source_repo.scm_instance().get_commit() - except EmptyRepositoryError: - h.flash(h.literal(_('There are no commits yet')), - category='warning') - redirect(h.route_path('repo_summary', repo_name=source_repo.repo_name)) - - commit_id = request.GET.get('commit') - branch_ref = request.GET.get('branch') - bookmark_ref = request.GET.get('bookmark') - - try: - source_repo_data = PullRequestModel().generate_repo_data( - source_repo, commit_id=commit_id, - branch=branch_ref, bookmark=bookmark_ref) - except CommitDoesNotExistError as e: - log.exception(e) - h.flash(_('Commit does not exist'), 'error') - redirect(url('pullrequest_home', repo_name=source_repo.repo_name)) - - default_target_repo = source_repo - - if source_repo.parent: - parent_vcs_obj = source_repo.parent.scm_instance() - if parent_vcs_obj and not parent_vcs_obj.is_empty(): - # change default if we have a parent repo - default_target_repo = source_repo.parent - - target_repo_data = PullRequestModel().generate_repo_data( - default_target_repo) - - selected_source_ref = source_repo_data['refs']['selected_ref'] - - title_source_ref = selected_source_ref.split(':', 2)[1] - c.default_title = PullRequestModel().generate_pullrequest_title( - source=source_repo.repo_name, - source_ref=title_source_ref, - target=default_target_repo.repo_name - ) - - c.default_repo_data = { - 'source_repo_name': source_repo.repo_name, - 'source_refs_json': json.dumps(source_repo_data), - 'target_repo_name': default_target_repo.repo_name, - 'target_refs_json': json.dumps(target_repo_data), - } - c.default_source_ref = selected_source_ref - - return render('/pullrequests/pullrequest.mako') - - @LoginRequired() - @NotAnonymous() - @XHRRequired() - @HasRepoPermissionAnyDecorator('repository.read', 'repository.write', - 'repository.admin') - @jsonify - def get_repo_refs(self, repo_name, target_repo_name): - repo = Repository.get_by_repo_name(target_repo_name) - if not repo: - raise HTTPNotFound - return PullRequestModel().generate_repo_data(repo) - - @LoginRequired() - @NotAnonymous() - @XHRRequired() - @HasRepoPermissionAnyDecorator('repository.read', 'repository.write', - 'repository.admin') - @jsonify - def get_repo_destinations(self, repo_name): - repo = Repository.get_by_repo_name(repo_name) - if not repo: - raise HTTPNotFound - filter_query = request.GET.get('query') - - query = Repository.query() \ - .order_by(func.length(Repository.repo_name)) \ - .filter(or_( - Repository.repo_name == repo.repo_name, - Repository.fork_id == repo.repo_id)) - - if filter_query: - ilike_expression = u'%{}%'.format(safe_unicode(filter_query)) - query = query.filter( - Repository.repo_name.ilike(ilike_expression)) - - add_parent = False - if repo.parent: - if filter_query in repo.parent.repo_name: - parent_vcs_obj = repo.parent.scm_instance() - if parent_vcs_obj and not parent_vcs_obj.is_empty(): - add_parent = True - - limit = 20 - 1 if add_parent else 20 - all_repos = query.limit(limit).all() - if add_parent: - all_repos += [repo.parent] - - repos = [] - for obj in self.scm_model.get_repos(all_repos): - repos.append({ - 'id': obj['name'], - 'text': obj['name'], - 'type': 'repo', - 'obj': obj['dbrepo'] - }) - - data = { - 'more': False, - 'results': [{ - 'text': _('Repositories'), - 'children': repos - }] if repos else [] - } - return data - - @LoginRequired() - @NotAnonymous() - @HasRepoPermissionAnyDecorator('repository.read', 'repository.write', - 'repository.admin') - @HasAcceptedRepoType('git', 'hg') - @auth.CSRFRequired() - def create(self, repo_name): - repo = Repository.get_by_repo_name(repo_name) - if not repo: - raise HTTPNotFound - - controls = peppercorn.parse(request.POST.items()) - - try: - _form = PullRequestForm(repo.repo_id)().to_python(controls) - except formencode.Invalid as errors: - if errors.error_dict.get('revisions'): - msg = 'Revisions: %s' % errors.error_dict['revisions'] - elif errors.error_dict.get('pullrequest_title'): - msg = _('Pull request requires a title with min. 3 chars') - else: - msg = _('Error creating pull request: {}').format(errors) - log.exception(msg) - h.flash(msg, 'error') - - # would rather just go back to form ... - return redirect(url('pullrequest_home', repo_name=repo_name)) - - source_repo = _form['source_repo'] - source_ref = _form['source_ref'] - target_repo = _form['target_repo'] - target_ref = _form['target_ref'] - commit_ids = _form['revisions'][::-1] - - # find the ancestor for this pr - source_db_repo = Repository.get_by_repo_name(_form['source_repo']) - target_db_repo = Repository.get_by_repo_name(_form['target_repo']) - - source_scm = source_db_repo.scm_instance() - target_scm = target_db_repo.scm_instance() - - source_commit = source_scm.get_commit(source_ref.split(':')[-1]) - target_commit = target_scm.get_commit(target_ref.split(':')[-1]) - - ancestor = source_scm.get_common_ancestor( - source_commit.raw_id, target_commit.raw_id, target_scm) - - target_ref_type, target_ref_name, __ = _form['target_ref'].split(':') - target_ref = ':'.join((target_ref_type, target_ref_name, ancestor)) - - pullrequest_title = _form['pullrequest_title'] - title_source_ref = source_ref.split(':', 2)[1] - if not pullrequest_title: - pullrequest_title = PullRequestModel().generate_pullrequest_title( - source=source_repo, - source_ref=title_source_ref, - target=target_repo - ) - - description = _form['pullrequest_desc'] - - get_default_reviewers_data, validate_default_reviewers = \ - PullRequestModel().get_reviewer_functions() - - # recalculate reviewers logic, to make sure we can validate this - reviewer_rules = get_default_reviewers_data( - c.rhodecode_user.get_instance(), source_db_repo, - source_commit, target_db_repo, target_commit) - - given_reviewers = _form['review_members'] - reviewers = validate_default_reviewers(given_reviewers, reviewer_rules) - - try: - pull_request = PullRequestModel().create( - c.rhodecode_user.user_id, source_repo, source_ref, target_repo, - target_ref, commit_ids, reviewers, pullrequest_title, - description, reviewer_rules - ) - Session().commit() - h.flash(_('Successfully opened new pull request'), - category='success') - except Exception as e: - msg = _('Error occurred during creation of this pull request.') - log.exception(msg) - h.flash(msg, category='error') - return redirect(url('pullrequest_home', repo_name=repo_name)) - - raise HTTPFound( - h.route_path('pullrequest_show', repo_name=target_repo, - pull_request_id=pull_request.pull_request_id)) - - @LoginRequired() - @NotAnonymous() - @HasRepoPermissionAnyDecorator('repository.read', 'repository.write', - 'repository.admin') - @auth.CSRFRequired() - @jsonify - def update(self, repo_name, pull_request_id): - pull_request_id = safe_int(pull_request_id) - pull_request = PullRequest.get_or_404(pull_request_id) - # only owner or admin can update it - allowed_to_update = PullRequestModel().check_user_update( - pull_request, c.rhodecode_user) - if allowed_to_update: - controls = peppercorn.parse(request.POST.items()) - - if 'review_members' in controls: - self._update_reviewers( - pull_request_id, controls['review_members'], - pull_request.reviewer_data) - elif str2bool(request.POST.get('update_commits', 'false')): - self._update_commits(pull_request) - elif str2bool(request.POST.get('edit_pull_request', 'false')): - self._edit_pull_request(pull_request) - else: - raise HTTPBadRequest() - return True - raise HTTPForbidden() - - def _edit_pull_request(self, pull_request): - try: - PullRequestModel().edit( - pull_request, request.POST.get('title'), - request.POST.get('description'), c.rhodecode_user) - except ValueError: - msg = _(u'Cannot update closed pull requests.') - h.flash(msg, category='error') - return - else: - Session().commit() - - msg = _(u'Pull request title & description updated.') - h.flash(msg, category='success') - return - - def _update_commits(self, pull_request): - resp = PullRequestModel().update_commits(pull_request) - - if resp.executed: - - if resp.target_changed and resp.source_changed: - changed = 'target and source repositories' - elif resp.target_changed and not resp.source_changed: - changed = 'target repository' - elif not resp.target_changed and resp.source_changed: - changed = 'source repository' - else: - changed = 'nothing' - - msg = _( - u'Pull request updated to "{source_commit_id}" with ' - u'{count_added} added, {count_removed} removed commits. ' - u'Source of changes: {change_source}') - msg = msg.format( - source_commit_id=pull_request.source_ref_parts.commit_id, - count_added=len(resp.changes.added), - count_removed=len(resp.changes.removed), - change_source=changed) - h.flash(msg, category='success') - - registry = get_current_registry() - rhodecode_plugins = getattr(registry, 'rhodecode_plugins', {}) - channelstream_config = rhodecode_plugins.get('channelstream', {}) - if channelstream_config.get('enabled'): - message = msg + ( - ' - ' - '{}'.format(_('Reload page'))) - channel = '/repo${}$/pr/{}'.format( - pull_request.target_repo.repo_name, - pull_request.pull_request_id - ) - payload = { - 'type': 'message', - 'user': 'system', - 'exclude_users': [request.user.username], - 'channel': channel, - 'message': { - 'message': message, - 'level': 'success', - 'topic': '/notifications' - } - } - channelstream_request( - channelstream_config, [payload], '/message', - raise_exc=False) - else: - msg = PullRequestModel.UPDATE_STATUS_MESSAGES[resp.reason] - warning_reasons = [ - UpdateFailureReason.NO_CHANGE, - UpdateFailureReason.WRONG_REF_TYPE, - ] - category = 'warning' if resp.reason in warning_reasons else 'error' - h.flash(msg, category=category) - - @auth.CSRFRequired() - @LoginRequired() - @NotAnonymous() - @HasRepoPermissionAnyDecorator('repository.read', 'repository.write', - 'repository.admin') - def merge(self, repo_name, pull_request_id): - """ - POST /{repo_name}/pull-request/{pull_request_id} - - Merge will perform a server-side merge of the specified - pull request, if the pull request is approved and mergeable. - After successful merging, the pull request is automatically - closed, with a relevant comment. - """ - pull_request_id = safe_int(pull_request_id) - pull_request = PullRequest.get_or_404(pull_request_id) - user = c.rhodecode_user - - check = MergeCheck.validate(pull_request, user) - merge_possible = not check.failed - - for err_type, error_msg in check.errors: - h.flash(error_msg, category=err_type) - - if merge_possible: - log.debug("Pre-conditions checked, trying to merge.") - extras = vcs_operation_context( - request.environ, repo_name=pull_request.target_repo.repo_name, - username=user.username, action='push', - scm=pull_request.target_repo.repo_type) - self._merge_pull_request(pull_request, user, extras) - - raise HTTPFound( - h.route_path('pullrequest_show', - repo_name=pull_request.target_repo.repo_name, - pull_request_id=pull_request.pull_request_id)) - - def _merge_pull_request(self, pull_request, user, extras): - merge_resp = PullRequestModel().merge( - pull_request, user, extras=extras) - - if merge_resp.executed: - log.debug("The merge was successful, closing the pull request.") - PullRequestModel().close_pull_request( - pull_request.pull_request_id, user) - Session().commit() - msg = _('Pull request was successfully merged and closed.') - h.flash(msg, category='success') - else: - log.debug( - "The merge was not successful. Merge response: %s", - merge_resp) - msg = PullRequestModel().merge_status_message( - merge_resp.failure_reason) - h.flash(msg, category='error') - - def _update_reviewers(self, pull_request_id, review_members, reviewer_rules): - - get_default_reviewers_data, validate_default_reviewers = \ - PullRequestModel().get_reviewer_functions() - - try: - reviewers = validate_default_reviewers(review_members, reviewer_rules) - except ValueError as e: - log.error('Reviewers Validation: {}'.format(e)) - h.flash(e, category='error') - return - - PullRequestModel().update_reviewers( - pull_request_id, reviewers, c.rhodecode_user) - h.flash(_('Pull request reviewers updated.'), category='success') - Session().commit() - - @LoginRequired() - @NotAnonymous() - @HasRepoPermissionAnyDecorator('repository.read', 'repository.write', - 'repository.admin') - @auth.CSRFRequired() - @jsonify - def delete(self, repo_name, pull_request_id): - pull_request_id = safe_int(pull_request_id) - pull_request = PullRequest.get_or_404(pull_request_id) - - pr_closed = pull_request.is_closed() - allowed_to_delete = PullRequestModel().check_user_delete( - pull_request, c.rhodecode_user) and not pr_closed - - # only owner can delete it ! - if allowed_to_delete: - PullRequestModel().delete(pull_request, c.rhodecode_user) - Session().commit() - h.flash(_('Successfully deleted pull request'), - category='success') - return redirect(url('my_account_pullrequests')) - - h.flash(_('Your are not allowed to delete this pull request'), - category='error') - raise HTTPForbidden() - - def _get_pr_version(self, pull_request_id, version=None): - pull_request_id = safe_int(pull_request_id) - at_version = None - - if version and version == 'latest': - pull_request_ver = PullRequest.get(pull_request_id) - pull_request_obj = pull_request_ver - _org_pull_request_obj = pull_request_obj - at_version = 'latest' - elif version: - pull_request_ver = PullRequestVersion.get_or_404(version) - pull_request_obj = pull_request_ver - _org_pull_request_obj = pull_request_ver.pull_request - at_version = pull_request_ver.pull_request_version_id - else: - _org_pull_request_obj = pull_request_obj = PullRequest.get_or_404( - pull_request_id) - - pull_request_display_obj = PullRequest.get_pr_display_object( - pull_request_obj, _org_pull_request_obj) - - return _org_pull_request_obj, pull_request_obj, \ - pull_request_display_obj, at_version - - def _get_diffset( - self, source_repo, source_ref_id, target_ref_id, target_commit, - source_commit, diff_limit, file_limit, display_inline_comments): - vcs_diff = PullRequestModel().get_diff( - source_repo, source_ref_id, target_ref_id) - - diff_processor = diffs.DiffProcessor( - vcs_diff, format='newdiff', diff_limit=diff_limit, - file_limit=file_limit, show_full_diff=c.fulldiff) - - _parsed = diff_processor.prepare() - - def _node_getter(commit): - def get_node(fname): - try: - return commit.get_node(fname) - except NodeDoesNotExistError: - return None - - return get_node - - diffset = codeblocks.DiffSet( - repo_name=c.repo_name, - source_repo_name=c.source_repo.repo_name, - source_node_getter=_node_getter(target_commit), - target_node_getter=_node_getter(source_commit), - comments=display_inline_comments - ) - diffset = diffset.render_patchset( - _parsed, target_commit.raw_id, source_commit.raw_id) - - return diffset - - @LoginRequired() - @HasRepoPermissionAnyDecorator('repository.read', 'repository.write', - 'repository.admin') - def show(self, repo_name, pull_request_id): - pull_request_id = safe_int(pull_request_id) - version = request.GET.get('version') - from_version = request.GET.get('from_version') or version - merge_checks = request.GET.get('merge_checks') - c.fulldiff = str2bool(request.GET.get('fulldiff')) - - (pull_request_latest, - pull_request_at_ver, - pull_request_display_obj, - at_version) = self._get_pr_version( - pull_request_id, version=version) - pr_closed = pull_request_latest.is_closed() - - if pr_closed and (version or from_version): - # not allow to browse versions - return redirect(h.url('pullrequest_show', repo_name=repo_name, - pull_request_id=pull_request_id)) - - versions = pull_request_display_obj.versions() - - c.at_version = at_version - c.at_version_num = (at_version - if at_version and at_version != 'latest' - else None) - c.at_version_pos = ChangesetComment.get_index_from_version( - c.at_version_num, versions) - - (prev_pull_request_latest, - prev_pull_request_at_ver, - prev_pull_request_display_obj, - prev_at_version) = self._get_pr_version( - pull_request_id, version=from_version) - - c.from_version = prev_at_version - c.from_version_num = (prev_at_version - if prev_at_version and prev_at_version != 'latest' - else None) - c.from_version_pos = ChangesetComment.get_index_from_version( - c.from_version_num, versions) - - # define if we're in COMPARE mode or VIEW at version mode - compare = at_version != prev_at_version - - # pull_requests repo_name we opened it against - # ie. target_repo must match - if repo_name != pull_request_at_ver.target_repo.repo_name: - raise HTTPNotFound - - c.shadow_clone_url = PullRequestModel().get_shadow_clone_url( - pull_request_at_ver) - - c.pull_request = pull_request_display_obj - c.pull_request_latest = pull_request_latest - - if compare or (at_version and not at_version == 'latest'): - c.allowed_to_change_status = False - c.allowed_to_update = False - c.allowed_to_merge = False - c.allowed_to_delete = False - c.allowed_to_comment = False - c.allowed_to_close = False - else: - can_change_status = PullRequestModel().check_user_change_status( - pull_request_at_ver, c.rhodecode_user) - c.allowed_to_change_status = can_change_status and not pr_closed - - c.allowed_to_update = PullRequestModel().check_user_update( - pull_request_latest, c.rhodecode_user) and not pr_closed - c.allowed_to_merge = PullRequestModel().check_user_merge( - pull_request_latest, c.rhodecode_user) and not pr_closed - c.allowed_to_delete = PullRequestModel().check_user_delete( - pull_request_latest, c.rhodecode_user) and not pr_closed - c.allowed_to_comment = not pr_closed - c.allowed_to_close = c.allowed_to_merge and not pr_closed - - c.forbid_adding_reviewers = False - c.forbid_author_to_review = False - c.forbid_commit_author_to_review = False - - if pull_request_latest.reviewer_data and \ - 'rules' in pull_request_latest.reviewer_data: - rules = pull_request_latest.reviewer_data['rules'] or {} - try: - c.forbid_adding_reviewers = rules.get( - 'forbid_adding_reviewers') - c.forbid_author_to_review = rules.get( - 'forbid_author_to_review') - c.forbid_commit_author_to_review = rules.get( - 'forbid_commit_author_to_review') - except Exception: - pass - - # check merge capabilities - _merge_check = MergeCheck.validate( - pull_request_latest, user=c.rhodecode_user) - c.pr_merge_errors = _merge_check.error_details - c.pr_merge_possible = not _merge_check.failed - c.pr_merge_message = _merge_check.merge_msg - - c.pull_request_review_status = _merge_check.review_status - if merge_checks: - return render('/pullrequests/pullrequest_merge_checks.mako') - - comments_model = CommentsModel() - - # reviewers and statuses - c.pull_request_reviewers = pull_request_at_ver.reviewers_statuses() - allowed_reviewers = [x[0].user_id for x in c.pull_request_reviewers] - - # GENERAL COMMENTS with versions # - q = comments_model._all_general_comments_of_pull_request(pull_request_latest) - q = q.order_by(ChangesetComment.comment_id.asc()) - general_comments = q - - # pick comments we want to render at current version - c.comment_versions = comments_model.aggregate_comments( - general_comments, versions, c.at_version_num) - c.comments = c.comment_versions[c.at_version_num]['until'] - - # INLINE COMMENTS with versions # - q = comments_model._all_inline_comments_of_pull_request(pull_request_latest) - q = q.order_by(ChangesetComment.comment_id.asc()) - inline_comments = q - - c.inline_versions = comments_model.aggregate_comments( - inline_comments, versions, c.at_version_num, inline=True) - - # inject latest version - latest_ver = PullRequest.get_pr_display_object( - pull_request_latest, pull_request_latest) - - c.versions = versions + [latest_ver] - - # if we use version, then do not show later comments - # than current version - display_inline_comments = collections.defaultdict( - lambda: collections.defaultdict(list)) - for co in inline_comments: - if c.at_version_num: - # pick comments that are at least UPTO given version, so we - # don't render comments for higher version - should_render = co.pull_request_version_id and \ - co.pull_request_version_id <= c.at_version_num - else: - # showing all, for 'latest' - should_render = True - - if should_render: - display_inline_comments[co.f_path][co.line_no].append(co) - - # load diff data into template context, if we use compare mode then - # diff is calculated based on changes between versions of PR - - source_repo = pull_request_at_ver.source_repo - source_ref_id = pull_request_at_ver.source_ref_parts.commit_id - - target_repo = pull_request_at_ver.target_repo - target_ref_id = pull_request_at_ver.target_ref_parts.commit_id - - if compare: - # in compare switch the diff base to latest commit from prev version - target_ref_id = prev_pull_request_display_obj.revisions[0] - - # despite opening commits for bookmarks/branches/tags, we always - # convert this to rev to prevent changes after bookmark or branch change - c.source_ref_type = 'rev' - c.source_ref = source_ref_id - - c.target_ref_type = 'rev' - c.target_ref = target_ref_id - - c.source_repo = source_repo - c.target_repo = target_repo - - # diff_limit is the old behavior, will cut off the whole diff - # if the limit is applied otherwise will just hide the - # big files from the front-end - diff_limit = self.cut_off_limit_diff - file_limit = self.cut_off_limit_file - - c.commit_ranges = [] - source_commit = EmptyCommit() - target_commit = EmptyCommit() - c.missing_requirements = False - - source_scm = source_repo.scm_instance() - target_scm = target_repo.scm_instance() - - # try first shadow repo, fallback to regular repo - try: - commits_source_repo = pull_request_latest.get_shadow_repo() - except Exception: - log.debug('Failed to get shadow repo', exc_info=True) - commits_source_repo = source_scm - - c.commits_source_repo = commits_source_repo - commit_cache = {} - try: - pre_load = ["author", "branch", "date", "message"] - show_revs = pull_request_at_ver.revisions - for rev in show_revs: - comm = commits_source_repo.get_commit( - commit_id=rev, pre_load=pre_load) - c.commit_ranges.append(comm) - commit_cache[comm.raw_id] = comm - - # Order here matters, we first need to get target, and then - # the source - target_commit = commits_source_repo.get_commit( - commit_id=safe_str(target_ref_id)) - - source_commit = commits_source_repo.get_commit( - commit_id=safe_str(source_ref_id)) - - except CommitDoesNotExistError: - log.warning( - 'Failed to get commit from `{}` repo'.format( - commits_source_repo), exc_info=True) - except RepositoryRequirementError: - log.warning( - 'Failed to get all required data from repo', exc_info=True) - c.missing_requirements = True - - c.ancestor = None # set it to None, to hide it from PR view - - try: - ancestor_id = source_scm.get_common_ancestor( - source_commit.raw_id, target_commit.raw_id, target_scm) - c.ancestor_commit = source_scm.get_commit(ancestor_id) - except Exception: - c.ancestor_commit = None - - c.statuses = source_repo.statuses( - [x.raw_id for x in c.commit_ranges]) - - # auto collapse if we have more than limit - collapse_limit = diffs.DiffProcessor._collapse_commits_over - c.collapse_all_commits = len(c.commit_ranges) > collapse_limit - c.compare_mode = compare - - c.missing_commits = False - if (c.missing_requirements or isinstance(source_commit, EmptyCommit) - or source_commit == target_commit): - - c.missing_commits = True - else: - - c.diffset = self._get_diffset( - commits_source_repo, source_ref_id, target_ref_id, - target_commit, source_commit, - diff_limit, file_limit, display_inline_comments) - - c.limited_diff = c.diffset.limited_diff - - # calculate removed files that are bound to comments - comment_deleted_files = [ - fname for fname in display_inline_comments - if fname not in c.diffset.file_stats] - - c.deleted_files_comments = collections.defaultdict(dict) - for fname, per_line_comments in display_inline_comments.items(): - if fname in comment_deleted_files: - c.deleted_files_comments[fname]['stats'] = 0 - c.deleted_files_comments[fname]['comments'] = list() - for lno, comments in per_line_comments.items(): - c.deleted_files_comments[fname]['comments'].extend( - comments) - - # this is a hack to properly display links, when creating PR, the - # compare view and others uses different notation, and - # compare_commits.mako renders links based on the target_repo. - # We need to swap that here to generate it properly on the html side - c.target_repo = c.source_repo - - c.commit_statuses = ChangesetStatus.STATUSES - - c.show_version_changes = not pr_closed - if c.show_version_changes: - cur_obj = pull_request_at_ver - prev_obj = prev_pull_request_at_ver - - old_commit_ids = prev_obj.revisions - new_commit_ids = cur_obj.revisions - commit_changes = PullRequestModel()._calculate_commit_id_changes( - old_commit_ids, new_commit_ids) - c.commit_changes_summary = commit_changes - - # calculate the diff for commits between versions - c.commit_changes = [] - mark = lambda cs, fw: list( - h.itertools.izip_longest([], cs, fillvalue=fw)) - for c_type, raw_id in mark(commit_changes.added, 'a') \ - + mark(commit_changes.removed, 'r') \ - + mark(commit_changes.common, 'c'): - - if raw_id in commit_cache: - commit = commit_cache[raw_id] - else: - try: - commit = commits_source_repo.get_commit(raw_id) - except CommitDoesNotExistError: - # in case we fail extracting still use "dummy" commit - # for display in commit diff - commit = h.AttributeDict( - {'raw_id': raw_id, - 'message': 'EMPTY or MISSING COMMIT'}) - c.commit_changes.append([c_type, commit]) - - # current user review statuses for each version - c.review_versions = {} - if c.rhodecode_user.user_id in allowed_reviewers: - for co in general_comments: - if co.author.user_id == c.rhodecode_user.user_id: - # each comment has a status change - status = co.status_change - if status: - _ver_pr = status[0].comment.pull_request_version_id - c.review_versions[_ver_pr] = status[0] - - return render('/pullrequests/pullrequest_show.mako') - - @LoginRequired() - @NotAnonymous() - @HasRepoPermissionAnyDecorator( - 'repository.read', 'repository.write', 'repository.admin') - @auth.CSRFRequired() - @jsonify - def comment(self, repo_name, pull_request_id): - pull_request_id = safe_int(pull_request_id) - pull_request = PullRequest.get_or_404(pull_request_id) - if pull_request.is_closed(): - log.debug('comment: forbidden because pull request is closed') - raise HTTPForbidden() - - status = request.POST.get('changeset_status', None) - text = request.POST.get('text') - comment_type = request.POST.get('comment_type') - resolves_comment_id = request.POST.get('resolves_comment_id', None) - close_pull_request = request.POST.get('close_pull_request') - - # the logic here should work like following, if we submit close - # pr comment, use `close_pull_request_with_comment` function - # else handle regular comment logic - user = c.rhodecode_user - repo = c.rhodecode_db_repo - - if close_pull_request: - # only owner or admin or person with write permissions - allowed_to_close = PullRequestModel().check_user_update( - pull_request, c.rhodecode_user) - if not allowed_to_close: - log.debug('comment: forbidden because not allowed to close ' - 'pull request %s', pull_request_id) - raise HTTPForbidden() - comment, status = PullRequestModel().close_pull_request_with_comment( - pull_request, user, repo, message=text) - Session().flush() - events.trigger( - events.PullRequestCommentEvent(pull_request, comment)) - - else: - # regular comment case, could be inline, or one with status. - # for that one we check also permissions - - allowed_to_change_status = PullRequestModel().check_user_change_status( - pull_request, c.rhodecode_user) - - if status and allowed_to_change_status: - message = (_('Status change %(transition_icon)s %(status)s') - % {'transition_icon': '>', - 'status': ChangesetStatus.get_status_lbl(status)}) - text = text or message - - comment = CommentsModel().create( - text=text, - repo=c.rhodecode_db_repo.repo_id, - user=c.rhodecode_user.user_id, - pull_request=pull_request_id, - f_path=request.POST.get('f_path'), - line_no=request.POST.get('line'), - status_change=(ChangesetStatus.get_status_lbl(status) - if status and allowed_to_change_status else None), - status_change_type=(status - if status and allowed_to_change_status else None), - comment_type=comment_type, - resolves_comment_id=resolves_comment_id - ) - - if allowed_to_change_status: - # calculate old status before we change it - old_calculated_status = pull_request.calculated_review_status() - - # get status if set ! - if status: - ChangesetStatusModel().set_status( - c.rhodecode_db_repo.repo_id, - status, - c.rhodecode_user.user_id, - comment, - pull_request=pull_request_id - ) - - Session().flush() - events.trigger( - events.PullRequestCommentEvent(pull_request, comment)) - - # we now calculate the status of pull request, and based on that - # calculation we set the commits status - calculated_status = pull_request.calculated_review_status() - if old_calculated_status != calculated_status: - PullRequestModel()._trigger_pull_request_hook( - pull_request, c.rhodecode_user, 'review_status_change') - - Session().commit() - - if not request.is_xhr: - raise HTTPFound( - h.route_path('pullrequest_show', - repo_name=repo_name, - pull_request_id=pull_request_id)) - - data = { - 'target_id': h.safeid(h.safe_unicode(request.POST.get('f_path'))), - } - if comment: - c.co = comment - rendered_comment = render('changeset/changeset_comment_block.mako') - data.update(comment.get_dict()) - data.update({'rendered_text': rendered_comment}) - - return data - - @LoginRequired() - @NotAnonymous() - @HasRepoPermissionAnyDecorator('repository.read', 'repository.write', - 'repository.admin') - @auth.CSRFRequired() - @jsonify - def delete_comment(self, repo_name, comment_id): - comment = ChangesetComment.get_or_404(comment_id) - if not comment: - log.debug('Comment with id:%s not found, skipping', comment_id) - # comment already deleted in another call probably - return True - - if comment.pull_request.is_closed(): - # don't allow deleting comments on closed pull request - raise HTTPForbidden() - - is_repo_admin = h.HasRepoPermissionAny('repository.admin')(c.repo_name) - super_admin = h.HasPermissionAny('hg.admin')() - comment_owner = comment.author.user_id == c.rhodecode_user.user_id - is_repo_comment = comment.repo.repo_name == c.repo_name - comment_repo_admin = is_repo_admin and is_repo_comment - - if super_admin or comment_owner or comment_repo_admin: - old_calculated_status = comment.pull_request.calculated_review_status() - CommentsModel().delete(comment=comment, user=c.rhodecode_user) - Session().commit() - calculated_status = comment.pull_request.calculated_review_status() - if old_calculated_status != calculated_status: - PullRequestModel()._trigger_pull_request_hook( - comment.pull_request, c.rhodecode_user, 'review_status_change') - return True - else: - log.warning('No permissions for user %s to delete comment_id: %s', - c.rhodecode_user, comment_id) - raise HTTPNotFound() diff --git a/rhodecode/model/comment.py b/rhodecode/model/comment.py --- a/rhodecode/model/comment.py +++ b/rhodecode/model/comment.py @@ -298,12 +298,11 @@ class CommentsModel(BaseModel): pr_target_repo = pull_request_obj.target_repo pr_source_repo = pull_request_obj.source_repo - pr_comment_url = h.url( + pr_comment_url = h.route_url( 'pullrequest_show', repo_name=pr_target_repo.repo_name, pull_request_id=pull_request_obj.pull_request_id, - anchor='comment-%s' % comment.comment_id, - qualified=True,) + anchor='comment-%s' % comment.comment_id) # set some variables for email notification pr_target_repo_url = h.route_url( diff --git a/rhodecode/model/pull_request.py b/rhodecode/model/pull_request.py --- a/rhodecode/model/pull_request.py +++ b/rhodecode/model/pull_request.py @@ -1541,6 +1541,7 @@ class MergeCheck(object): if fail_early: return merge_check + log.debug('MergeCheck: is failed: %s', merge_check.failed) return merge_check diff --git a/rhodecode/public/js/rhodecode/routes.js b/rhodecode/public/js/rhodecode/routes.js --- a/rhodecode/public/js/rhodecode/routes.js +++ b/rhodecode/public/js/rhodecode/routes.js @@ -15,14 +15,6 @@ function registerRCRoutes() { pyroutes.register('new_repo', '/_admin/create_repository', []); pyroutes.register('edit_user', '/_admin/users/%(user_id)s/edit', ['user_id']); pyroutes.register('edit_user_group_members', '/_admin/user_groups/%(user_group_id)s/edit/members', ['user_group_id']); - pyroutes.register('pullrequest_home', '/%(repo_name)s/pull-request/new', ['repo_name']); - pyroutes.register('pullrequest', '/%(repo_name)s/pull-request/new', ['repo_name']); - pyroutes.register('pullrequest_repo_refs', '/%(repo_name)s/pull-request/refs/%(target_repo_name)s', ['repo_name', 'target_repo_name']); - pyroutes.register('pullrequest_repo_destinations', '/%(repo_name)s/pull-request/repo-destinations', ['repo_name']); - pyroutes.register('pullrequest_show', '/%(repo_name)s/pull-request/%(pull_request_id)s', ['repo_name', 'pull_request_id']); - pyroutes.register('pullrequest_update', '/%(repo_name)s/pull-request/%(pull_request_id)s', ['repo_name', 'pull_request_id']); - pyroutes.register('pullrequest_comment', '/%(repo_name)s/pull-request-comment/%(pull_request_id)s', ['repo_name', 'pull_request_id']); - pyroutes.register('pullrequest_comment_delete', '/%(repo_name)s/pull-request-comment/%(comment_id)s/delete', ['repo_name', 'comment_id']); pyroutes.register('favicon', '/favicon.ico', []); pyroutes.register('robots', '/robots.txt', []); pyroutes.register('auth_home', '/_admin/auth*traverse', []); @@ -152,6 +144,15 @@ function registerRCRoutes() { pyroutes.register('pullrequest_show', '/%(repo_name)s/pull-request/%(pull_request_id)s', ['repo_name', 'pull_request_id']); pyroutes.register('pullrequest_show_all', '/%(repo_name)s/pull-request', ['repo_name']); pyroutes.register('pullrequest_show_all_data', '/%(repo_name)s/pull-request-data', ['repo_name']); + pyroutes.register('pullrequest_repo_refs', '/%(repo_name)s/pull-request/refs/%(target_repo_name)s', ['repo_name', 'target_repo_name']); + pyroutes.register('pullrequest_repo_destinations', '/%(repo_name)s/pull-request/repo-destinations', ['repo_name']); + pyroutes.register('pullrequest_new', '/%(repo_name)s/pull-request/new', ['repo_name']); + pyroutes.register('pullrequest_create', '/%(repo_name)s/pull-request/create', ['repo_name']); + pyroutes.register('pullrequest_update', '/%(repo_name)s/pull-request/%(pull_request_id)s/update', ['repo_name', 'pull_request_id']); + pyroutes.register('pullrequest_merge', '/%(repo_name)s/pull-request/%(pull_request_id)s/merge', ['repo_name', 'pull_request_id']); + pyroutes.register('pullrequest_delete', '/%(repo_name)s/pull-request/%(pull_request_id)s/delete', ['repo_name', 'pull_request_id']); + pyroutes.register('pullrequest_comment_create', '/%(repo_name)s/pull-request/%(pull_request_id)s/comment', ['repo_name', 'pull_request_id']); + pyroutes.register('pullrequest_comment_delete', '/%(repo_name)s/pull-request/%(pull_request_id)s/comment/%(comment_id)s/delete', ['repo_name', 'pull_request_id', 'comment_id']); pyroutes.register('edit_repo', '/%(repo_name)s/settings', ['repo_name']); pyroutes.register('edit_repo_advanced', '/%(repo_name)s/settings/advanced', ['repo_name']); pyroutes.register('edit_repo_advanced_delete', '/%(repo_name)s/settings/advanced/delete', ['repo_name']); diff --git a/rhodecode/public/js/src/rhodecode/comments.js b/rhodecode/public/js/src/rhodecode/comments.js --- a/rhodecode/public/js/src/rhodecode/comments.js +++ b/rhodecode/public/js/src/rhodecode/comments.js @@ -138,7 +138,7 @@ var bindToggleButtons = function() { 'commit_id': this.commitId}); } else if (this.pullRequestId) { - this.submitUrl = pyroutes.url('pullrequest_comment', + this.submitUrl = pyroutes.url('pullrequest_comment_create', {'repo_name': templateContext.repo_name, 'pull_request_id': this.pullRequestId}); this.selfUrl = pyroutes.url('pullrequest_show', diff --git a/rhodecode/public/js/src/rhodecode/pullrequests.js b/rhodecode/public/js/src/rhodecode/pullrequests.js --- a/rhodecode/public/js/src/rhodecode/pullrequests.js +++ b/rhodecode/public/js/src/rhodecode/pullrequests.js @@ -387,7 +387,6 @@ var editPullRequest = function(repo_name {"repo_name": repo_name, "pull_request_id": pull_request_id}); var postData = { - '_method': 'put', 'title': title, 'description': description, 'edit_pull_request': true, diff --git a/rhodecode/templates/base/base.mako b/rhodecode/templates/base/base.mako --- a/rhodecode/templates/base/base.mako +++ b/rhodecode/templates/base/base.mako @@ -276,7 +276,7 @@ %if c.rhodecode_user.username != h.DEFAULT_USER: %if c.rhodecode_db_repo.repo_type in ['git','hg']:
  • ${_('Fork')}
  • -
  • ${_('Create Pull Request')}
  • +
  • ${_('Create Pull Request')}
  • %endif %endif diff --git a/rhodecode/templates/changelog/changelog.mako b/rhodecode/templates/changelog/changelog.mako --- a/rhodecode/templates/changelog/changelog.mako +++ b/rhodecode/templates/changelog/changelog.mako @@ -55,7 +55,7 @@ ## pr open link %if h.is_hg(c.rhodecode_repo) or h.is_git(c.rhodecode_repo): - + ${_('Open new pull request')} @@ -176,7 +176,7 @@ .show(); $commitRangeClear.show(); - var _url = pyroutes.url('pullrequest_home', + var _url = pyroutes.url('pullrequest_new', {'repo_name': '${c.repo_name}', 'commit': revEnd}); open_new_pull_request.attr('href', _url); @@ -186,12 +186,12 @@ $commitRangeClear.hide(); %if c.branch_name: - var _url = pyroutes.url('pullrequest_home', + var _url = pyroutes.url('pullrequest_new', {'repo_name': '${c.repo_name}', 'branch':'${c.branch_name}'}); open_new_pull_request.attr('href', _url); %else: - var _url = pyroutes.url('pullrequest_home', + var _url = pyroutes.url('pullrequest_new', {'repo_name': '${c.repo_name}'}); open_new_pull_request.attr('href', _url); %endif diff --git a/rhodecode/templates/files/files_delete.mako b/rhodecode/templates/files/files_delete.mako --- a/rhodecode/templates/files/files_delete.mako +++ b/rhodecode/templates/files/files_delete.mako @@ -27,7 +27,7 @@
    ${self.breadcrumbs()}
    - ${h.secure_form(h.route_path('repo_files_delete_file', repo_name=c.repo_name, commit_id=c.commit.raw_id, f_path=c.f_path), id='eform', method='POST', class_="form-horizontal")} + ${h.secure_form(h.route_path('repo_files_delete_file', repo_name=c.repo_name, commit_id=c.commit.raw_id, f_path=c.f_path), id='eform', method='POST', class_="form-horizontal", request=request)}
    diff --git a/rhodecode/templates/files/files_edit.mako b/rhodecode/templates/files/files_edit.mako --- a/rhodecode/templates/files/files_edit.mako +++ b/rhodecode/templates/files/files_edit.mako @@ -42,7 +42,7 @@
    - ${h.secure_form(h.route_path('repo_files_update_file', repo_name=c.repo_name, commit_id=c.commit.raw_id, f_path=c.f_path), id='eform', method='POST')} + ${h.secure_form(h.route_path('repo_files_update_file', repo_name=c.repo_name, commit_id=c.commit.raw_id, f_path=c.f_path), id='eform', method='POST', request=request)}
    diff --git a/rhodecode/templates/pullrequests/pullrequest.mako b/rhodecode/templates/pullrequests/pullrequest.mako --- a/rhodecode/templates/pullrequests/pullrequest.mako +++ b/rhodecode/templates/pullrequests/pullrequest.mako @@ -22,7 +22,7 @@ ${self.repo_page_title(c.rhodecode_db_repo)}
    - ${h.secure_form(h.url('pullrequest', repo_name=c.repo_name), method='post', id='pull_request_form')} + ${h.secure_form(h.route_path('pullrequest_create', repo_name=c.repo_name), id='pull_request_form', method='POST', request=request)} ${self.breadcrumbs()} diff --git a/rhodecode/templates/pullrequests/pullrequest_merge_checks.mako b/rhodecode/templates/pullrequests/pullrequest_merge_checks.mako --- a/rhodecode/templates/pullrequests/pullrequest_merge_checks.mako +++ b/rhodecode/templates/pullrequests/pullrequest_merge_checks.mako @@ -32,7 +32,7 @@
    % if c.allowed_to_merge:
    - ${h.secure_form(h.url('pullrequest_merge', repo_name=c.repo_name, pull_request_id=c.pull_request.pull_request_id), id='merge_pull_request_form')} + ${h.secure_form(h.route_path('pullrequest_merge', repo_name=c.repo_name, pull_request_id=c.pull_request.pull_request_id), id='merge_pull_request_form', method='POST', request=request)} <% merge_disabled = ' disabled' if c.pr_merge_possible is False else '' %> ${_('refresh checks')} diff --git a/rhodecode/templates/pullrequests/pullrequest_show.mako b/rhodecode/templates/pullrequests/pullrequest_show.mako --- a/rhodecode/templates/pullrequests/pullrequest_show.mako +++ b/rhodecode/templates/pullrequests/pullrequest_show.mako @@ -32,7 +32,7 @@
    @@ -52,7 +52,7 @@ %if c.allowed_to_update:
    % if c.allowed_to_delete: - ${h.secure_form(h.url('pullrequest_delete', repo_name=c.pull_request.target_repo.repo_name, pull_request_id=c.pull_request.pull_request_id),method='delete')} + ${h.secure_form(h.route_path('pullrequest_delete', repo_name=c.pull_request.target_repo.repo_name, pull_request_id=c.pull_request.pull_request_id), method='POST', request=request)} ${h.submit('remove_%s' % c.pull_request.pull_request_id, _('Delete'), class_="btn btn-link btn-danger no-margin",onclick="return confirm('"+_('Confirm to delete this pull request')+"');")} ${h.end_form()} @@ -200,7 +200,7 @@ - v${ver_pos} + v${ver_pos} @@ -616,8 +616,8 @@
    ## main comment form and it status - ${comment.comments(h.url('pullrequest_comment', repo_name=c.repo_name, - pull_request_id=c.pull_request.pull_request_id), + ${comment.comments(h.route_path('pullrequest_comment_create', repo_name=c.repo_name, + pull_request_id=c.pull_request.pull_request_id), c.pull_request_review_status, is_pull_request=True, change_status=c.allowed_to_change_status)} %endif @@ -730,7 +730,7 @@ }; refreshMergeChecks = function(){ - var loadUrl = "${h.url.current(merge_checks=1)}"; + var loadUrl = "${request.current_route_path(_query=dict(merge_checks=1))}"; $('.pull-request-merge').css('opacity', 0.3); $('.action-buttons-extra').css('opacity', 0.3); diff --git a/rhodecode/templates/pullrequests/pullrequests.mako b/rhodecode/templates/pullrequests/pullrequests.mako --- a/rhodecode/templates/pullrequests/pullrequests.mako +++ b/rhodecode/templates/pullrequests/pullrequests.mako @@ -30,7 +30,7 @@
  • %if c.rhodecode_user.username != h.DEFAULT_USER: - + ${_('Open new Pull Request')} diff --git a/rhodecode/tests/controllers/test_pullrequests.py b/rhodecode/tests/controllers/test_pullrequests.py deleted file mode 100644 --- a/rhodecode/tests/controllers/test_pullrequests.py +++ /dev/null @@ -1,42 +0,0 @@ -# -*- coding: utf-8 -*- - -# Copyright (C) 2016-2017 RhodeCode GmbH -# -# This program is free software: you can redistribute it and/or modify -# it under the terms of the GNU Affero General Public License, version 3 -# (only), as published by the Free Software Foundation. -# -# This program is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU General Public License for more details. -# -# You should have received a copy of the GNU Affero General Public License -# along with this program. If not, see . -# -# This program is dual-licensed. If you wish to learn more about the -# RhodeCode Enterprise Edition, including its added features, Support services, -# and proprietary license terms, please see https://rhodecode.com/licenses/ - -import mock - -from rhodecode.controllers import pullrequests -from rhodecode.lib.vcs.backends.base import ( - MergeFailureReason, MergeResponse) -from rhodecode.model.pull_request import PullRequestModel -from rhodecode.tests import assert_session_flash - - -def test_merge_pull_request_renders_failure_reason(app, user_regular): - pull_request = mock.Mock() - controller = pullrequests.PullrequestsController() - model_patcher = mock.patch.multiple( - PullRequestModel, - merge=mock.Mock(return_value=MergeResponse( - True, False, 'STUB_COMMIT_ID', MergeFailureReason.PUSH_FAILED)), - merge_status=mock.Mock(return_value=(True, 'WRONG_MESSAGE'))) - with model_patcher: - controller._merge_pull_request(pull_request, user_regular, extras={}) - - assert_session_flash(msg=PullRequestModel.MERGE_STATUS_MESSAGES[ - MergeFailureReason.PUSH_FAILED]) diff --git a/rhodecode/tests/functional/test_admin_user_groups.py b/rhodecode/tests/functional/test_admin_user_groups.py --- a/rhodecode/tests/functional/test_admin_user_groups.py +++ b/rhodecode/tests/functional/test_admin_user_groups.py @@ -200,13 +200,14 @@ class TestAdminUsersGroupsController(Tes assert response.body == '{"members": []}' fixture.destroy_user_group(TEST_USER_GROUP) - def test_usergroup_escape(self): - user = User.get_by_username('test_admin') - user.name = '' - user.lastname = ( - '') - Session().add(user) - Session().commit() + def test_usergroup_escape(self, user_util): + user = user_util.create_user( + username='escape_user', + firstname='', + lastname='' + ) + + user_util.create_user_group(owner=user.username) self.log_user() users_group_name = 'samplegroup'