##// END OF EJS Templates
pull-requests: add explicit CLOSE pr action instead of closed status from selector....
marcink -
r1445:934edf37 default
parent child Browse files
Show More
@@ -69,6 +69,8 b' class PullrequestsController(BaseRepoCon'
69
69
70 def __before__(self):
70 def __before__(self):
71 super(PullrequestsController, self).__before__()
71 super(PullrequestsController, self).__before__()
72 c.REVIEW_STATUS_APPROVED = ChangesetStatus.STATUS_APPROVED
73 c.REVIEW_STATUS_REJECTED = ChangesetStatus.STATUS_REJECTED
72
74
73 def _extract_ordering(self, request):
75 def _extract_ordering(self, request):
74 column_index = safe_int(request.GET.get('order[0][column]'))
76 column_index = safe_int(request.GET.get('order[0][column]'))
@@ -686,9 +688,12 b' class PullrequestsController(BaseRepoCon'
686 c.allowed_to_merge = False
688 c.allowed_to_merge = False
687 c.allowed_to_delete = False
689 c.allowed_to_delete = False
688 c.allowed_to_comment = False
690 c.allowed_to_comment = False
691 c.allowed_to_close = False
689 else:
692 else:
690 c.allowed_to_change_status = PullRequestModel(). \
693 c.allowed_to_change_status = PullRequestModel(). \
691 check_user_change_status(pull_request_at_ver, c.rhodecode_user)
694 check_user_change_status(pull_request_at_ver, c.rhodecode_user) \
695 and not pr_closed
696
692 c.allowed_to_update = PullRequestModel().check_user_update(
697 c.allowed_to_update = PullRequestModel().check_user_update(
693 pull_request_latest, c.rhodecode_user) and not pr_closed
698 pull_request_latest, c.rhodecode_user) and not pr_closed
694 c.allowed_to_merge = PullRequestModel().check_user_merge(
699 c.allowed_to_merge = PullRequestModel().check_user_merge(
@@ -696,6 +701,7 b' class PullrequestsController(BaseRepoCon'
696 c.allowed_to_delete = PullRequestModel().check_user_delete(
701 c.allowed_to_delete = PullRequestModel().check_user_delete(
697 pull_request_latest, c.rhodecode_user) and not pr_closed
702 pull_request_latest, c.rhodecode_user) and not pr_closed
698 c.allowed_to_comment = not pr_closed
703 c.allowed_to_comment = not pr_closed
704 c.allowed_to_close = c.allowed_to_change_status and not pr_closed
699
705
700 # check merge capabilities
706 # check merge capabilities
701 _merge_check = MergeCheck.validate(
707 _merge_check = MergeCheck.validate(
@@ -704,6 +710,7 b' class PullrequestsController(BaseRepoCon'
704 c.pr_merge_possible = not _merge_check.failed
710 c.pr_merge_possible = not _merge_check.failed
705 c.pr_merge_message = _merge_check.merge_msg
711 c.pr_merge_message = _merge_check.merge_msg
706
712
713 c.pull_request_review_status = _merge_check.review_status
707 if merge_checks:
714 if merge_checks:
708 return render('/pullrequests/pullrequest_merge_checks.mako')
715 return render('/pullrequests/pullrequest_merge_checks.mako')
709
716
@@ -712,7 +719,6 b' class PullrequestsController(BaseRepoCon'
712 # reviewers and statuses
719 # reviewers and statuses
713 c.pull_request_reviewers = pull_request_at_ver.reviewers_statuses()
720 c.pull_request_reviewers = pull_request_at_ver.reviewers_statuses()
714 allowed_reviewers = [x[0].user_id for x in c.pull_request_reviewers]
721 allowed_reviewers = [x[0].user_id for x in c.pull_request_reviewers]
715 c.pull_request_review_status = pull_request_at_ver.calculated_review_status()
716
722
717 # GENERAL COMMENTS with versions #
723 # GENERAL COMMENTS with versions #
718 q = comments_model._all_general_comments_of_pull_request(pull_request_latest)
724 q = comments_model._all_general_comments_of_pull_request(pull_request_latest)
@@ -860,12 +866,7 b' class PullrequestsController(BaseRepoCon'
860 # We need to swap that here to generate it properly on the html side
866 # We need to swap that here to generate it properly on the html side
861 c.target_repo = c.source_repo
867 c.target_repo = c.source_repo
862
868
863 if c.allowed_to_update:
869 c.commit_statuses = ChangesetStatus.STATUSES
864 force_close = ('forced_closed', _('Close Pull Request'))
865 statuses = ChangesetStatus.STATUSES + [force_close]
866 else:
867 statuses = ChangesetStatus.STATUSES
868 c.commit_statuses = statuses
869
870
870 c.show_version_changes = not pr_closed
871 c.show_version_changes = not pr_closed
871 if c.show_version_changes:
872 if c.show_version_changes:
@@ -924,22 +925,21 b' class PullrequestsController(BaseRepoCon'
924 if pull_request.is_closed():
925 if pull_request.is_closed():
925 raise HTTPForbidden()
926 raise HTTPForbidden()
926
927
927 # TODO: johbo: Re-think this bit, "approved_closed" does not exist
928 # as a changeset status, still we want to send it in one value.
929 status = request.POST.get('changeset_status', None)
928 status = request.POST.get('changeset_status', None)
930 text = request.POST.get('text')
929 text = request.POST.get('text')
931 comment_type = request.POST.get('comment_type')
930 comment_type = request.POST.get('comment_type')
932 resolves_comment_id = request.POST.get('resolves_comment_id', None)
931 resolves_comment_id = request.POST.get('resolves_comment_id', None)
932 close_pull_request = request.POST.get('close_pull_request')
933
933
934 if status and '_closed' in status:
934 close_pr = False
935 if close_pull_request:
935 close_pr = True
936 close_pr = True
936 status = status.replace('_closed', '')
937 pull_request_review_status = pull_request.calculated_review_status()
938 if pull_request_review_status == ChangesetStatus.STATUS_APPROVED:
939 # approved only if we have voting consent
940 status = ChangesetStatus.STATUS_APPROVED
937 else:
941 else:
938 close_pr = False
942 status = ChangesetStatus.STATUS_REJECTED
939
940 forced = (status == 'forced')
941 if forced:
942 status = 'rejected'
943
943
944 allowed_to_change_status = PullRequestModel().check_user_change_status(
944 allowed_to_change_status = PullRequestModel().check_user_change_status(
945 pull_request, c.rhodecode_user)
945 pull_request, c.rhodecode_user)
@@ -995,7 +995,7 b' class PullrequestsController(BaseRepoCon'
995 status_completed = (
995 status_completed = (
996 calculated_status in [ChangesetStatus.STATUS_APPROVED,
996 calculated_status in [ChangesetStatus.STATUS_APPROVED,
997 ChangesetStatus.STATUS_REJECTED])
997 ChangesetStatus.STATUS_REJECTED])
998 if forced or status_completed:
998 if close_pull_request or status_completed:
999 PullRequestModel().close_pull_request(
999 PullRequestModel().close_pull_request(
1000 pull_request_id, c.rhodecode_user)
1000 pull_request_id, c.rhodecode_user)
1001 else:
1001 else:
@@ -1333,6 +1333,7 b' class MergeCheck(object):'
1333 MERGE_CHECK = 'merge'
1333 MERGE_CHECK = 'merge'
1334
1334
1335 def __init__(self):
1335 def __init__(self):
1336 self.review_status = None
1336 self.merge_possible = None
1337 self.merge_possible = None
1337 self.merge_msg = ''
1338 self.merge_msg = ''
1338 self.failed = None
1339 self.failed = None
@@ -1355,7 +1356,7 b' class MergeCheck(object):'
1355
1356
1356 merge_check = cls()
1357 merge_check = cls()
1357
1358
1358 # permissions
1359 # permissions to merge
1359 user_allowed_to_merge = PullRequestModel().check_user_merge(
1360 user_allowed_to_merge = PullRequestModel().check_user_merge(
1360 pull_request, user)
1361 pull_request, user)
1361 if not user_allowed_to_merge:
1362 if not user_allowed_to_merge:
@@ -1366,8 +1367,10 b' class MergeCheck(object):'
1366 if fail_early:
1367 if fail_early:
1367 return merge_check
1368 return merge_check
1368
1369
1369 # review status
1370 # review status, must be always present
1370 review_status = pull_request.calculated_review_status()
1371 review_status = pull_request.calculated_review_status()
1372 merge_check.review_status = review_status
1373
1371 status_approved = review_status == ChangesetStatus.STATUS_APPROVED
1374 status_approved = review_status == ChangesetStatus.STATUS_APPROVED
1372 if not status_approved:
1375 if not status_approved:
1373 log.debug("MergeCheck: cannot merge, approval is pending.")
1376 log.debug("MergeCheck: cannot merge, approval is pending.")
@@ -174,6 +174,19 b' input[type="button"] {'
174 }
174 }
175 }
175 }
176
176
177 .btn-approved-status {
178 .border ( @border-thickness, @alert1 );
179 background-color: white;
180 color: @alert1;
181
182 }
183
184 .btn-rejected-status {
185 .border ( @border-thickness, @alert2 );
186 background-color: white;
187 color: @alert2;
188 }
189
177 .btn-sm,
190 .btn-sm,
178 .btn-mini,
191 .btn-mini,
179 .field-sm .btn {
192 .field-sm .btn {
@@ -384,6 +384,10 b' form.comment-form {'
384 float: right;
384 float: right;
385 display: inline-block;
385 display: inline-block;
386 }
386 }
387
388 .action-buttons-extra {
389 display: inline-block;
390 }
387 }
391 }
388
392
389 .comment-form {
393 .comment-form {
@@ -92,6 +92,8 b' var bindToggleButtons = function() {'
92 this.resolvesId = null;
92 this.resolvesId = null;
93 this.resolvesActionId = null;
93 this.resolvesActionId = null;
94
94
95 this.closesPr = '#close_pull_request';
96
95 this.cmBox = this.withLineNo('#text');
97 this.cmBox = this.withLineNo('#text');
96 this.cm = initCommentBoxCodeMirror(this, this.cmBox, this.initAutocompleteActions);
98 this.cm = initCommentBoxCodeMirror(this, this.cmBox, this.initAutocompleteActions);
97
99
@@ -175,6 +177,11 b' var bindToggleButtons = function() {'
175 this.getResolvesId = function() {
177 this.getResolvesId = function() {
176 return $(this.submitForm).find(this.resolvesId).val() || null;
178 return $(this.submitForm).find(this.resolvesId).val() || null;
177 };
179 };
180
181 this.getClosePr = function() {
182 return $(this.submitForm).find(this.closesPr).val() || null;
183 };
184
178 this.markCommentResolved = function(resolvedCommentId){
185 this.markCommentResolved = function(resolvedCommentId){
179 $('#comment-label-{0}'.format(resolvedCommentId)).find('.resolved').show();
186 $('#comment-label-{0}'.format(resolvedCommentId)).find('.resolved').show();
180 $('#comment-label-{0}'.format(resolvedCommentId)).find('.resolve').hide();
187 $('#comment-label-{0}'.format(resolvedCommentId)).find('.resolve').hide();
@@ -209,6 +216,7 b' var bindToggleButtons = function() {'
209 });
216 });
210 $(this.submitForm).find(this.statusChange).on('change', function() {
217 $(this.submitForm).find(this.statusChange).on('change', function() {
211 var status = self.getCommentStatus();
218 var status = self.getCommentStatus();
219
212 if (status && !self.isInline()) {
220 if (status && !self.isInline()) {
213 $(self.submitButton).prop('disabled', false);
221 $(self.submitButton).prop('disabled', false);
214 }
222 }
@@ -236,6 +244,8 b' var bindToggleButtons = function() {'
236 // destroy the resolve action
244 // destroy the resolve action
237 $(this.resolvesId).parent().remove();
245 $(this.resolvesId).parent().remove();
238 }
246 }
247 // reset closingPR flag
248 $('.close-pr-input').remove();
239
249
240 $(this.statusChange).select2('readonly', false);
250 $(this.statusChange).select2('readonly', false);
241 };
251 };
@@ -284,6 +294,7 b' var bindToggleButtons = function() {'
284 var status = self.getCommentStatus();
294 var status = self.getCommentStatus();
285 var commentType = self.getCommentType();
295 var commentType = self.getCommentType();
286 var resolvesCommentId = self.getResolvesId();
296 var resolvesCommentId = self.getResolvesId();
297 var closePullRequest = self.getClosePr();
287
298
288 if (text === "" && !status) {
299 if (text === "" && !status) {
289 return;
300 return;
@@ -300,10 +311,15 b' var bindToggleButtons = function() {'
300 'comment_type': commentType,
311 'comment_type': commentType,
301 'csrf_token': CSRF_TOKEN
312 'csrf_token': CSRF_TOKEN
302 };
313 };
314
303 if (resolvesCommentId){
315 if (resolvesCommentId) {
304 postData['resolves_comment_id'] = resolvesCommentId;
316 postData['resolves_comment_id'] = resolvesCommentId;
305 }
317 }
306
318
319 if (closePullRequest) {
320 postData['close_pull_request'] = true;
321 }
322
307 var submitSuccessCallback = function(o) {
323 var submitSuccessCallback = function(o) {
308 // reload page if we change status for single commit.
324 // reload page if we change status for single commit.
309 if (status && self.commitId) {
325 if (status && self.commitId) {
@@ -359,6 +375,7 b' var bindToggleButtons = function() {'
359 // submit button, but only on Main form, isInline means inline
375 // submit button, but only on Main form, isInline means inline
360 submitState = false
376 submitState = false
361 }
377 }
378
362 $(this.submitButton).prop('disabled', submitState);
379 $(this.submitButton).prop('disabled', submitState);
363 if (submitEvent) {
380 if (submitEvent) {
364 $(this.submitButton).val(_gettext('Submitting...'));
381 $(this.submitButton).val(_gettext('Submitting...'));
@@ -385,6 +385,11 b''
385 ${_('Cancel')}
385 ${_('Cancel')}
386 </button>
386 </button>
387 % endif
387 % endif
388
389 % if form_type != 'inline':
390 <div class="action-buttons-extra"></div>
391 % endif
392
388 ${h.submit('save', _('Comment'), class_='btn btn-success comment-button-input')}
393 ${h.submit('save', _('Comment'), class_='btn btn-success comment-button-input')}
389
394
390 </div>
395 </div>
@@ -1,7 +1,6 b''
1
1
2 <div class="pull-request-wrap">
2 <div class="pull-request-wrap">
3
3
4
5 % if c.pr_merge_possible:
4 % if c.pr_merge_possible:
6 <h2 class="merge-status">
5 <h2 class="merge-status">
7 <span class="merge-icon success"><i class="icon-true"></i></span>
6 <span class="merge-icon success"><i class="icon-true"></i></span>
@@ -46,5 +45,19 b''
46 <input type="submit" value="${_('Login to Merge this Pull Request')}" class="btn disabled" disabled="disabled">
45 <input type="submit" value="${_('Login to Merge this Pull Request')}" class="btn disabled" disabled="disabled">
47 % endif
46 % endif
48 </div>
47 </div>
48
49 % if c.allowed_to_close:
50 ## close PR action, injected later next to COMMENT button
51 <div id="close-pull-request-action" style="display: none">
52 % if c.pull_request_review_status == c.REVIEW_STATUS_APPROVED:
53 <a class="btn btn-approved-status" href="#close-as-approved" onclick="closePullRequest('${c.REVIEW_STATUS_APPROVED}'); return false;">
54 ${_('Close with status {}').format(h.commit_status_lbl(c.REVIEW_STATUS_APPROVED))}
55 </a>
56 % else:
57 <a class="btn btn-rejected-status" href="#close-as-rejected" onclick="closePullRequest('${c.REVIEW_STATUS_REJECTED}'); return false;">
58 ${_('Close with status {}').format(h.commit_status_lbl(c.REVIEW_STATUS_REJECTED))}
59 </a>
60 % endif
49 </div>
61 </div>
50
62 % endif
63 </div>
@@ -694,13 +694,33 b''
694 refreshMergeChecks = function(){
694 refreshMergeChecks = function(){
695 var loadUrl = "${h.url.current(merge_checks=1)}";
695 var loadUrl = "${h.url.current(merge_checks=1)}";
696 $('.pull-request-merge').css('opacity', 0.3);
696 $('.pull-request-merge').css('opacity', 0.3);
697 $('.action-buttons-extra').css('opacity', 0.3);
698
697 $('.pull-request-merge').load(
699 $('.pull-request-merge').load(
698 loadUrl,function() {
700 loadUrl, function() {
699 $('.pull-request-merge').css('opacity', 1);
701 $('.pull-request-merge').css('opacity', 1);
702
703 $('.action-buttons-extra').css('opacity', 1);
704 injectCloseAction();
700 }
705 }
701 );
706 );
702 };
707 };
703
708
709 injectCloseAction = function() {
710 var closeAction = $('#close-pull-request-action').html();
711 var $actionButtons = $('.action-buttons-extra');
712 // clear the action before
713 $actionButtons.html("");
714 $actionButtons.html(closeAction);
715 };
716
717 closePullRequest = function (status) {
718 // inject closing flag
719 $('.action-buttons-extra').append('<input type="hidden" class="close-pr-input" id="close_pull_request" value="1">');
720 $(generalCommentForm.statusChange).select2("val", status).trigger('change');
721 $(generalCommentForm.submitForm).submit();
722 };
723
704 $('#show-outdated-comments').on('click', function(e){
724 $('#show-outdated-comments').on('click', function(e){
705 var button = $(this);
725 var button = $(this);
706 var outdated = $('.comment-outdated');
726 var outdated = $('.comment-outdated');
@@ -786,6 +806,8 b''
786 window.commentFormGlobalSubmitSuccessCallback = function(){
806 window.commentFormGlobalSubmitSuccessCallback = function(){
787 refreshMergeChecks();
807 refreshMergeChecks();
788 };
808 };
809 // initial injection
810 injectCloseAction();
789
811
790 })
812 })
791 </script>
813 </script>
@@ -115,8 +115,10 b' class TestPullrequestsController:'
115 repo_name=pull_request.target_repo.scm_instance().name,
115 repo_name=pull_request.target_repo.scm_instance().name,
116 pull_request_id=str(pull_request.pull_request_id)))
116 pull_request_id=str(pull_request.pull_request_id)))
117
117
118 assert 'Server-side pull request merging is disabled.' in response
118 response.mustcontain('Server-side pull request merging is disabled.')
119 assert 'value="forced_closed"' in response
119
120 assert_response = response.assert_response()
121 assert_response.one_element_exists('#close-pull-request-action')
120
122
121 def test_show_invalid_commit_id(self, pr_util):
123 def test_show_invalid_commit_id(self, pr_util):
122 # Simulating invalid revisions which will cause a lookup error
124 # Simulating invalid revisions which will cause a lookup error
@@ -242,10 +244,9 b' class TestPullrequestsController:'
242 repo_name=pull_request.target_repo.scm_instance().name,
244 repo_name=pull_request.target_repo.scm_instance().name,
243 pull_request_id=str(pull_request_id)),
245 pull_request_id=str(pull_request_id)),
244 params={
246 params={
245 'changeset_status':
247 'changeset_status': ChangesetStatus.STATUS_APPROVED,
246 ChangesetStatus.STATUS_APPROVED + '_closed',
248 'close_pull_request': '1',
247 'change_changeset_status': 'on',
249 'text': 'Closing a PR',
248 'text': '',
249 'csrf_token': csrf_token},
250 'csrf_token': csrf_token},
250 status=302)
251 status=302)
251
252
@@ -257,6 +258,14 b' class TestPullrequestsController:'
257 .all()
258 .all()
258 assert len(journal) == 1
259 assert len(journal) == 1
259
260
261 pull_request = PullRequest.get(pull_request_id)
262 assert pull_request.is_closed()
263
264 # check only the latest status, not the review status
265 status = ChangesetStatusModel().get_status(
266 pull_request.source_repo, pull_request=pull_request)
267 assert status == ChangesetStatus.STATUS_APPROVED
268
260 def test_reject_and_close_pull_request(self, pr_util, csrf_token):
269 def test_reject_and_close_pull_request(self, pr_util, csrf_token):
261 pull_request = pr_util.create_pull_request()
270 pull_request = pr_util.create_pull_request()
262 pull_request_id = pull_request.pull_request_id
271 pull_request_id = pull_request.pull_request_id
@@ -291,7 +300,8 b' class TestPullrequestsController:'
291 repo_name=pull_request.target_repo.scm_instance().name,
300 repo_name=pull_request.target_repo.scm_instance().name,
292 pull_request_id=str(pull_request_id)),
301 pull_request_id=str(pull_request_id)),
293 params={
302 params={
294 'changeset_status': 'forced_closed',
303 'changeset_status': 'rejected',
304 'close_pull_request': '1',
295 'csrf_token': csrf_token},
305 'csrf_token': csrf_token},
296 status=302)
306 status=302)
297
307
General Comments 0
You need to be logged in to leave comments. Login now