# HG changeset patch # User Marcin Kuzminski # Date 2016-10-30 19:58:11 # Node ID a6c56473ce7fade8a16aad988fd0e22ff270a643 # Parent 84545e70b68976ea3b04b3a6971e18af3716a7bc pull-requests: moved the delete logic into the show view. - delete from pr view is more intuitive - there was several support queries about beeing confused how to delete a pr diff --git a/rhodecode/controllers/pullrequests.py b/rhodecode/controllers/pullrequests.py --- a/rhodecode/controllers/pullrequests.py +++ b/rhodecode/controllers/pullrequests.py @@ -687,6 +687,8 @@ class PullrequestsController(BaseRepoCon c.pull_request, c.rhodecode_user) and not c.pull_request.is_closed() c.shadow_clone_url = PullRequestModel().get_shadow_clone_url( c.pull_request) + c.allowed_to_delete = PullRequestModel().check_user_delete( + c.pull_request, c.rhodecode_user) and not c.pull_request.is_closed() cc_model = ChangesetCommentsModel() 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 @@ -149,6 +149,11 @@ class PullRequestModel(BaseModel): owner = user.user_id == pull_request.user_id return self.check_user_merge(pull_request, user, api) or owner + def check_user_delete(self, pull_request, user): + owner = user.user_id == pull_request.user_id + _perms = ('repository.admin') + return self._check_perms(_perms, pull_request, user) or owner + def check_user_change_status(self, pull_request, user, api=False): reviewer = user.user_id in [x.user_id for x in pull_request.reviewers] diff --git a/rhodecode/public/css/main.less b/rhodecode/public/css/main.less --- a/rhodecode/public/css/main.less +++ b/rhodecode/public/css/main.less @@ -1343,6 +1343,11 @@ table.integrations { .pr-details-title { padding-bottom: 8px; border-bottom: @border-thickness solid @grey5; + + .action_button.disabled { + color: @grey4; + cursor: inherit; + } .action_button { color: @rcblue; } diff --git a/rhodecode/templates/pullrequests/pullrequest_show.html b/rhodecode/templates/pullrequests/pullrequest_show.html --- a/rhodecode/templates/pullrequests/pullrequest_show.html +++ b/rhodecode/templates/pullrequests/pullrequest_show.html @@ -47,8 +47,18 @@
${_('Pull request #%s') % c.pull_request.pull_request_id} ${_('From')} ${h.format_date(c.pull_request.created_on)} %if c.allowed_to_update: - ${_('Edit')} - +
+ % if c.allowed_to_delete: + ${h.secure_form(url('pullrequest_delete', repo_name=c.pull_request.target_repo.repo_name, pull_request_id=c.pull_request.pull_request_id),method='delete')} + ${h.submit('remove_%s' % c.pull_request.pull_request_id, _('Delete'), + class_="btn btn-link btn-danger",onclick="return confirm('"+_('Confirm to delete this pull request')+"');")} + ${h.end_form()} + % else: + ${_('Delete')} + % endif +
+
${_('Edit')}
+ %endif
@@ -457,6 +467,7 @@ var PRDetails = { editButton: $('#open_edit_pullrequest'), closeButton: $('#close_edit_pullrequest'), + deleteButton: $('#delete_pullrequest'), viewFields: $('#pr-desc, #pr-title'), editFields: $('#pr-desc-edit, #pr-title-edit, #pr-save'), @@ -469,11 +480,15 @@ edit: function(event) { this.viewFields.hide(); this.editButton.hide(); + this.deleteButton.hide(); + this.closeButton.show(); this.editFields.show(); codeMirrorInstance.refresh(); }, view: function(event) { + this.editButton.show(); + this.deleteButton.show(); this.editFields.hide(); this.closeButton.hide(); this.viewFields.show(); @@ -504,7 +519,7 @@ this.closeButton.hide(); this.addButton.hide(); this.removeButtons.css('visibility', 'hidden'); - } + }, }; PRDetails.init(); @@ -603,12 +618,12 @@ if(button.hasClass("comments-visible")) { $('#{0} .inline-comments'.format(boxid)).each(function(index){ $(this).hide(); - }) + }); button.removeClass("comments-visible"); } else { $('#{0} .inline-comments'.format(boxid)).each(function(index){ $(this).show(); - }) + }); button.addClass("comments-visible"); } }); diff --git a/rhodecode/tests/controllers/test_pullrequests.py b/rhodecode/tests/controllers/test_pullrequests.py --- a/rhodecode/tests/controllers/test_pullrequests.py +++ b/rhodecode/tests/controllers/test_pullrequests.py @@ -27,7 +27,7 @@ from rhodecode.model.pull_request import from rhodecode.tests import assert_session_flash -def test_merge_pull_request_renders_failure_reason(user_regular): +def test_merge_pull_request_renders_failure_reason(app, user_regular): pull_request = mock.Mock() controller = pullrequests.PullrequestsController() model_patcher = mock.patch.multiple( diff --git a/rhodecode/tests/functional/test_pullrequests.py b/rhodecode/tests/functional/test_pullrequests.py --- a/rhodecode/tests/functional/test_pullrequests.py +++ b/rhodecode/tests/functional/test_pullrequests.py @@ -30,6 +30,7 @@ from rhodecode.model.db import ( from rhodecode.model.meta import Session from rhodecode.model.pull_request import PullRequestModel from rhodecode.model.user import UserModel +from rhodecode.model.repo import RepoModel from rhodecode.tests import assert_session_flash, url, TEST_USER_ADMIN_LOGIN from rhodecode.tests.utils import AssertResponse @@ -966,6 +967,68 @@ class TestPullrequestsController: assertr.no_element_exists('div.pr-mergeinfo') +@pytest.mark.usefixtures('app') +@pytest.mark.backends("git", "hg") +class TestPullrequestsControllerDelete(object): + def test_pull_request_delete_button_permissions_admin( + self, autologin_user, user_admin, pr_util): + pull_request = pr_util.create_pull_request( + author=user_admin.username, enable_notifications=False) + + response = self.app.get(url( + controller='pullrequests', action='show', + repo_name=pull_request.target_repo.scm_instance().name, + pull_request_id=str(pull_request.pull_request_id))) + + response.mustcontain('id="delete_pullrequest"') + response.mustcontain('Confirm to delete this pull request') + + def test_pull_request_delete_button_permissions_owner( + self, autologin_regular_user, user_regular, pr_util): + pull_request = pr_util.create_pull_request( + author=user_regular.username, enable_notifications=False) + + response = self.app.get(url( + controller='pullrequests', action='show', + repo_name=pull_request.target_repo.scm_instance().name, + pull_request_id=str(pull_request.pull_request_id))) + + response.mustcontain('id="delete_pullrequest"') + response.mustcontain('Confirm to delete this pull request') + + def test_pull_request_delete_button_permissions_forbidden( + self, autologin_regular_user, user_regular, user_admin, pr_util): + pull_request = pr_util.create_pull_request( + author=user_admin.username, enable_notifications=False) + + response = self.app.get(url( + controller='pullrequests', action='show', + repo_name=pull_request.target_repo.scm_instance().name, + pull_request_id=str(pull_request.pull_request_id))) + response.mustcontain(no=['id="delete_pullrequest"']) + response.mustcontain(no=['Confirm to delete this pull request']) + + def test_pull_request_delete_button_permissions_can_update_cannot_delete( + self, autologin_regular_user, user_regular, user_admin, pr_util, + user_util): + + pull_request = pr_util.create_pull_request( + author=user_admin.username, enable_notifications=False) + + user_util.grant_user_permission_to_repo( + pull_request.target_repo, user_regular, + 'repository.write') + + response = self.app.get(url( + controller='pullrequests', action='show', + repo_name=pull_request.target_repo.scm_instance().name, + pull_request_id=str(pull_request.pull_request_id))) + + response.mustcontain('id="open_edit_pullrequest"') + response.mustcontain('id="delete_pullrequest"') + response.mustcontain(no=['Confirm to delete this pull request']) + + def assert_pull_request_status(pull_request, expected_status): status = ChangesetStatusModel().calculated_review_status( pull_request=pull_request)