##// END OF EJS Templates
pull-requests: unified merge checks....
marcink -
r1335:7ea0471c default
parent child Browse files
Show More
@@ -32,7 +32,7 b' from rhodecode.lib.utils2 import str2boo'
32 32 from rhodecode.model.changeset_status import ChangesetStatusModel
33 33 from rhodecode.model.comment import CommentsModel
34 34 from rhodecode.model.db import Session, ChangesetStatus
35 from rhodecode.model.pull_request import PullRequestModel
35 from rhodecode.model.pull_request import PullRequestModel, MergeCheck
36 36 from rhodecode.model.settings import SettingsModel
37 37
38 38 log = logging.getLogger(__name__)
@@ -270,13 +270,14 b' def merge_pull_request(request, apiuser,'
270 270 raise JSONRPCError('userid is not the same as your user')
271 271
272 272 pull_request = get_pull_request_or_error(pullrequestid)
273 if not PullRequestModel().check_user_merge(
274 pull_request, apiuser, api=True):
275 raise JSONRPCError('repository `%s` does not exist' % (repoid,))
276 if pull_request.is_closed():
273
274 check = MergeCheck.validate(pull_request, user=apiuser)
275 merge_possible = not check.failed
276
277 if not merge_possible:
278 reasons = ','.join([msg for _e, msg in check.errors])
277 279 raise JSONRPCError(
278 'pull request `%s` merge failed, pull request is closed' % (
279 pullrequestid,))
280 'merge not possible for following reasons: {}'.format(reasons))
280 281
281 282 target_repo = pull_request.target_repo
282 283 extras = vcs_operation_context(
@@ -60,7 +60,7 b' from rhodecode.model.db import (PullRequ'
60 60 Repository, PullRequestVersion)
61 61 from rhodecode.model.forms import PullRequestForm
62 62 from rhodecode.model.meta import Session
63 from rhodecode.model.pull_request import PullRequestModel
63 from rhodecode.model.pull_request import PullRequestModel, MergeCheck
64 64
65 65 log = logging.getLogger(__name__)
66 66
@@ -597,14 +597,20 b' class PullrequestsController(BaseRepoCon'
597 597
598 598 Merge will perform a server-side merge of the specified
599 599 pull request, if the pull request is approved and mergeable.
600 After succesfull merging, the pull request is automatically
600 After successful merging, the pull request is automatically
601 601 closed, with a relevant comment.
602 602 """
603 603 pull_request_id = safe_int(pull_request_id)
604 604 pull_request = PullRequest.get_or_404(pull_request_id)
605 605 user = c.rhodecode_user
606 606
607 if self._meets_merge_pre_conditions(pull_request, user):
607 check = MergeCheck.validate(pull_request, user)
608 merge_possible = not check.failed
609
610 for err_type, error_msg in check.errors:
611 h.flash(error_msg, category=err_type)
612
613 if merge_possible:
608 614 log.debug("Pre-conditions checked, trying to merge.")
609 615 extras = vcs_operation_context(
610 616 request.environ, repo_name=pull_request.target_repo.repo_name,
@@ -617,36 +623,6 b' class PullrequestsController(BaseRepoCon'
617 623 repo_name=pull_request.target_repo.repo_name,
618 624 pull_request_id=pull_request.pull_request_id))
619 625
620 def _meets_merge_pre_conditions(self, pull_request, user):
621 if not PullRequestModel().check_user_merge(pull_request, user):
622 raise HTTPForbidden()
623
624 merge_status, msg = PullRequestModel().merge_status(pull_request)
625 if not merge_status:
626 log.debug("Cannot merge, not mergeable.")
627 h.flash(msg, category='error')
628 return False
629
630 if (pull_request.calculated_review_status()
631 is not ChangesetStatus.STATUS_APPROVED):
632 log.debug("Cannot merge, approval is pending.")
633 msg = _('Pull request reviewer approval is pending.')
634 h.flash(msg, category='error')
635 return False
636
637 todos = CommentsModel().get_unresolved_todos(pull_request)
638 if todos:
639 log.debug("Cannot merge, unresolved todos left.")
640 if len(todos) == 1:
641 msg = _('Cannot merge, {} todo still not resolved.').format(
642 len(todos))
643 else:
644 msg = _('Cannot merge, {} todos still not resolved.').format(
645 len(todos))
646 h.flash(msg, category='error')
647 return False
648 return True
649
650 626 def _merge_pull_request(self, pull_request, user, extras):
651 627 merge_resp = PullRequestModel().merge(
652 628 pull_request, user, extras=extras)
@@ -855,24 +831,11 b' class PullrequestsController(BaseRepoCon'
855 831 if should_render:
856 832 display_inline_comments[co.f_path][co.line_no].append(co)
857 833
858 c.pr_merge_checks = []
859 c.pr_merge_status, c.pr_merge_msg = PullRequestModel().merge_status(
860 pull_request_at_ver)
861 c.pr_merge_checks.append(['warning' if not c.pr_merge_status else 'success', c.pr_merge_msg])
862
863 if c.pull_request_review_status != ChangesetStatus.STATUS_APPROVED:
864 approval_msg = _('Reviewer approval is pending.')
865 c.pr_merge_status = False
866 c.pr_merge_checks.append(['warning', approval_msg])
867
868 todos = cc_model.get_unresolved_todos(pull_request_latest)
869 if todos:
870 c.pr_merge_status = False
871 if len(todos) == 1:
872 msg = _('{} todo still not resolved.').format(len(todos))
873 else:
874 msg = _('{} todos still not resolved.').format(len(todos))
875 c.pr_merge_checks.append(['warning', msg])
834 _merge_check = MergeCheck.validate(
835 pull_request_latest, user=c.rhodecode_user)
836 c.pr_merge_errors = _merge_check.errors
837 c.pr_merge_possible = not _merge_check.failed
838 c.pr_merge_message = _merge_check.merge_msg
876 839
877 840 if merge_checks:
878 841 return render('/pullrequests/pullrequest_merge_checks.mako')
@@ -1311,6 +1311,86 b' class PullRequestModel(BaseModel):'
1311 1311 pull_request.target_repo)
1312 1312
1313 1313
1314 class MergeCheck(object):
1315 """
1316 Perform Merge Checks and returns a check object which stores information
1317 about merge errors, and merge conditions
1318 """
1319
1320 def __init__(self):
1321 self.merge_possible = None
1322 self.merge_msg = ''
1323 self.failed = None
1324 self.errors = []
1325
1326 def push_error(self, error_type, message):
1327 self.failed = True
1328 self.errors.append([error_type, message])
1329
1330 @classmethod
1331 def validate(cls, pull_request, user, fail_early=False, translator=None):
1332 # if migrated to pyramid...
1333 # _ = lambda: translator or _ # use passed in translator if any
1334
1335 merge_check = cls()
1336
1337 # permissions
1338 user_allowed_to_merge = PullRequestModel().check_user_merge(
1339 pull_request, user)
1340 if not user_allowed_to_merge:
1341 log.debug("MergeCheck: cannot merge, approval is pending.")
1342
1343 msg = _('User `{}` not allowed to perform merge').format(user)
1344 merge_check.push_error('error', msg)
1345 if fail_early:
1346 return merge_check
1347
1348 # review status
1349 review_status = pull_request.calculated_review_status()
1350 status_approved = review_status == ChangesetStatus.STATUS_APPROVED
1351 if not status_approved:
1352 log.debug("MergeCheck: cannot merge, approval is pending.")
1353
1354 msg = _('Pull request reviewer approval is pending.')
1355
1356 merge_check.push_error('warning', msg)
1357
1358 if fail_early:
1359 return merge_check
1360
1361 # left over TODOs
1362 todos = CommentsModel().get_unresolved_todos(pull_request)
1363 if todos:
1364 log.debug("MergeCheck: cannot merge, {} "
1365 "unresolved todos left.".format(len(todos)))
1366
1367 if len(todos) == 1:
1368 msg = _('Cannot merge, {} TODO still not resolved.').format(
1369 len(todos))
1370 else:
1371 msg = _('Cannot merge, {} TODOs still not resolved.').format(
1372 len(todos))
1373
1374 merge_check.push_error('warning', msg)
1375
1376 if fail_early:
1377 return merge_check
1378
1379 # merge possible
1380 merge_status, msg = PullRequestModel().merge_status(pull_request)
1381 merge_check.merge_possible = merge_status
1382 merge_check.merge_msg = msg
1383 if not merge_status:
1384 log.debug(
1385 "MergeCheck: cannot merge, pull request merge not possible.")
1386 merge_check.push_error('warning', msg)
1387
1388 if fail_early:
1389 return merge_check
1390
1391 return merge_check
1392
1393
1314 1394 ChangeTuple = namedtuple('ChangeTuple',
1315 1395 ['added', 'common', 'removed'])
1316 1396
@@ -1639,24 +1639,28 b' BIN_FILENODE = 7'
1639 1639 padding: 0px 0px;
1640 1640 }
1641 1641
1642 .merge-status {
1643 margin-right: 5px;
1644 }
1645
1642 1646 .merge-message {
1643 1647 font-size: 1.2em
1644 1648 }
1645 .merge-message li{
1646 text-decoration: none;
1647 }
1648 1649
1649 .merge-message.success i {
1650 .merge-message.success i,
1651 .merge-icon.success i {
1650 1652 color:@alert1;
1651 1653 }
1652 .merge-message.warning i {
1653 color: @alert3;
1654 }
1655 .merge-message.error i {
1656 color:@alert2;
1657 }
1658 1654
1655 .merge-message.warning i,
1656 .merge-icon.warning i {
1657 color: @alert3;
1658 }
1659 1659
1660 .merge-message.error i,
1661 .merge-icon.error i {
1662 color:@alert2;
1663 }
1660 1664
1661 1665 .pr-versions {
1662 1666 position: relative;
@@ -1,16 +1,24 b''
1 1
2 2 <div class="pull-request-wrap">
3 3
4
5 % if c.pr_merge_possible:
6 <h2 class="merge-status">
7 <span class="merge-icon success"><i class="icon-true"></i></span>
8 ${_('This pull request can be merged automatically.')}
9 </h2>
10 % else:
11 <h2 class="merge-status">
12 <span class="merge-icon warning"><i class="icon-false"></i></span>
13 ${_('Merge is not currently possible because of below failed checks.')}
14 </h2>
15 % endif
16
4 17 <ul>
5 % for pr_check_type, pr_check_msg in c.pr_merge_checks:
18 % for pr_check_type, pr_check_msg in c.pr_merge_errors:
6 19 <li>
7 20 <span class="merge-message ${pr_check_type}" data-role="merge-message">
8 % if pr_check_type in ['success']:
9 <i class="icon-true"></i>
10 % else:
11 <i class="icon-false"></i>
12 % endif
13 ${pr_check_msg}
21 - ${pr_check_msg}
14 22 </span>
15 23 </li>
16 24 % endfor
@@ -20,7 +28,7 b''
20 28 % if c.allowed_to_merge:
21 29 <div class="pull-right">
22 30 ${h.secure_form(url('pullrequest_merge', repo_name=c.repo_name, pull_request_id=c.pull_request.pull_request_id), id='merge_pull_request_form')}
23 <% merge_disabled = ' disabled' if c.pr_merge_status is False else '' %>
31 <% merge_disabled = ' disabled' if c.pr_merge_possible is False else '' %>
24 32 <a class="btn" href="#" onclick="refreshMergeChecks(); return false;">${_('refresh checks')}</a>
25 33 <input type="submit" id="merge_pull_request" value="${_('Merge Pull Request')}" class="btn${merge_disabled}"${merge_disabled}>
26 34 ${h.end_form()}
@@ -32,6 +40,5 b''
32 40 <input type="submit" value="${_('Login to Merge this Pull Request')}" class="btn disabled" disabled="disabled">
33 41 % endif
34 42 </div>
35
36 43 </div>
37 44
@@ -541,7 +541,9 b' class TestPullrequestsController:'
541 541 params={'csrf_token': csrf_token}).follow()
542 542
543 543 assert response.status_int == 200
544 assert 'Server-side pull request merging is disabled.' in response.body
544 response.mustcontain(
545 'Merge is not currently possible because of below failed checks.')
546 response.mustcontain('Server-side pull request merging is disabled.')
545 547
546 548 @pytest.mark.skip_backends('svn')
547 549 def test_merge_pull_request_not_approved(self, pr_util, csrf_token):
@@ -556,10 +558,11 b' class TestPullrequestsController:'
556 558 pull_request_id=str(pull_request_id)),
557 559 params={'csrf_token': csrf_token}).follow()
558 560
559 pull_request = PullRequest.get(pull_request_id)
561 assert response.status_int == 200
560 562
561 assert response.status_int == 200
562 assert ' Reviewer approval is pending.' in response.body
563 response.mustcontain(
564 'Merge is not currently possible because of below failed checks.')
565 response.mustcontain('Pull request reviewer approval is pending.')
563 566
564 567 def test_update_source_revision(self, backend, csrf_token):
565 568 commits = [
General Comments 0
You need to be logged in to leave comments. Login now