##// 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 from rhodecode.model.changeset_status import ChangesetStatusModel
32 from rhodecode.model.changeset_status import ChangesetStatusModel
33 from rhodecode.model.comment import CommentsModel
33 from rhodecode.model.comment import CommentsModel
34 from rhodecode.model.db import Session, ChangesetStatus
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 from rhodecode.model.settings import SettingsModel
36 from rhodecode.model.settings import SettingsModel
37
37
38 log = logging.getLogger(__name__)
38 log = logging.getLogger(__name__)
@@ -270,13 +270,14 b' def merge_pull_request(request, apiuser,'
270 raise JSONRPCError('userid is not the same as your user')
270 raise JSONRPCError('userid is not the same as your user')
271
271
272 pull_request = get_pull_request_or_error(pullrequestid)
272 pull_request = get_pull_request_or_error(pullrequestid)
273 if not PullRequestModel().check_user_merge(
273
274 pull_request, apiuser, api=True):
274 check = MergeCheck.validate(pull_request, user=apiuser)
275 raise JSONRPCError('repository `%s` does not exist' % (repoid,))
275 merge_possible = not check.failed
276 if pull_request.is_closed():
276
277 if not merge_possible:
278 reasons = ','.join([msg for _e, msg in check.errors])
277 raise JSONRPCError(
279 raise JSONRPCError(
278 'pull request `%s` merge failed, pull request is closed' % (
280 'merge not possible for following reasons: {}'.format(reasons))
279 pullrequestid,))
280
281
281 target_repo = pull_request.target_repo
282 target_repo = pull_request.target_repo
282 extras = vcs_operation_context(
283 extras = vcs_operation_context(
@@ -60,7 +60,7 b' from rhodecode.model.db import (PullRequ'
60 Repository, PullRequestVersion)
60 Repository, PullRequestVersion)
61 from rhodecode.model.forms import PullRequestForm
61 from rhodecode.model.forms import PullRequestForm
62 from rhodecode.model.meta import Session
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 log = logging.getLogger(__name__)
65 log = logging.getLogger(__name__)
66
66
@@ -597,14 +597,20 b' class PullrequestsController(BaseRepoCon'
597
597
598 Merge will perform a server-side merge of the specified
598 Merge will perform a server-side merge of the specified
599 pull request, if the pull request is approved and mergeable.
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 closed, with a relevant comment.
601 closed, with a relevant comment.
602 """
602 """
603 pull_request_id = safe_int(pull_request_id)
603 pull_request_id = safe_int(pull_request_id)
604 pull_request = PullRequest.get_or_404(pull_request_id)
604 pull_request = PullRequest.get_or_404(pull_request_id)
605 user = c.rhodecode_user
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 log.debug("Pre-conditions checked, trying to merge.")
614 log.debug("Pre-conditions checked, trying to merge.")
609 extras = vcs_operation_context(
615 extras = vcs_operation_context(
610 request.environ, repo_name=pull_request.target_repo.repo_name,
616 request.environ, repo_name=pull_request.target_repo.repo_name,
@@ -617,36 +623,6 b' class PullrequestsController(BaseRepoCon'
617 repo_name=pull_request.target_repo.repo_name,
623 repo_name=pull_request.target_repo.repo_name,
618 pull_request_id=pull_request.pull_request_id))
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 def _merge_pull_request(self, pull_request, user, extras):
626 def _merge_pull_request(self, pull_request, user, extras):
651 merge_resp = PullRequestModel().merge(
627 merge_resp = PullRequestModel().merge(
652 pull_request, user, extras=extras)
628 pull_request, user, extras=extras)
@@ -855,24 +831,11 b' class PullrequestsController(BaseRepoCon'
855 if should_render:
831 if should_render:
856 display_inline_comments[co.f_path][co.line_no].append(co)
832 display_inline_comments[co.f_path][co.line_no].append(co)
857
833
858 c.pr_merge_checks = []
834 _merge_check = MergeCheck.validate(
859 c.pr_merge_status, c.pr_merge_msg = PullRequestModel().merge_status(
835 pull_request_latest, user=c.rhodecode_user)
860 pull_request_at_ver)
836 c.pr_merge_errors = _merge_check.errors
861 c.pr_merge_checks.append(['warning' if not c.pr_merge_status else 'success', c.pr_merge_msg])
837 c.pr_merge_possible = not _merge_check.failed
862
838 c.pr_merge_message = _merge_check.merge_msg
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])
876
839
877 if merge_checks:
840 if merge_checks:
878 return render('/pullrequests/pullrequest_merge_checks.mako')
841 return render('/pullrequests/pullrequest_merge_checks.mako')
@@ -1311,6 +1311,86 b' class PullRequestModel(BaseModel):'
1311 pull_request.target_repo)
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 ChangeTuple = namedtuple('ChangeTuple',
1394 ChangeTuple = namedtuple('ChangeTuple',
1315 ['added', 'common', 'removed'])
1395 ['added', 'common', 'removed'])
1316
1396
@@ -1639,24 +1639,28 b' BIN_FILENODE = 7'
1639 padding: 0px 0px;
1639 padding: 0px 0px;
1640 }
1640 }
1641
1641
1642 .merge-status {
1643 margin-right: 5px;
1644 }
1645
1642 .merge-message {
1646 .merge-message {
1643 font-size: 1.2em
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 color:@alert1;
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 .pr-versions {
1665 .pr-versions {
1662 position: relative;
1666 position: relative;
@@ -1,16 +1,24 b''
1
1
2 <div class="pull-request-wrap">
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 <ul>
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 <li>
19 <li>
7 <span class="merge-message ${pr_check_type}" data-role="merge-message">
20 <span class="merge-message ${pr_check_type}" data-role="merge-message">
8 % if pr_check_type in ['success']:
21 - ${pr_check_msg}
9 <i class="icon-true"></i>
10 % else:
11 <i class="icon-false"></i>
12 % endif
13 ${pr_check_msg}
14 </span>
22 </span>
15 </li>
23 </li>
16 % endfor
24 % endfor
@@ -20,7 +28,7 b''
20 % if c.allowed_to_merge:
28 % if c.allowed_to_merge:
21 <div class="pull-right">
29 <div class="pull-right">
22 ${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')}
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 <a class="btn" href="#" onclick="refreshMergeChecks(); return false;">${_('refresh checks')}</a>
32 <a class="btn" href="#" onclick="refreshMergeChecks(); return false;">${_('refresh checks')}</a>
25 <input type="submit" id="merge_pull_request" value="${_('Merge Pull Request')}" class="btn${merge_disabled}"${merge_disabled}>
33 <input type="submit" id="merge_pull_request" value="${_('Merge Pull Request')}" class="btn${merge_disabled}"${merge_disabled}>
26 ${h.end_form()}
34 ${h.end_form()}
@@ -32,6 +40,5 b''
32 <input type="submit" value="${_('Login to Merge this Pull Request')}" class="btn disabled" disabled="disabled">
40 <input type="submit" value="${_('Login to Merge this Pull Request')}" class="btn disabled" disabled="disabled">
33 % endif
41 % endif
34 </div>
42 </div>
35
36 </div>
43 </div>
37
44
@@ -541,7 +541,9 b' class TestPullrequestsController:'
541 params={'csrf_token': csrf_token}).follow()
541 params={'csrf_token': csrf_token}).follow()
542
542
543 assert response.status_int == 200
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 @pytest.mark.skip_backends('svn')
548 @pytest.mark.skip_backends('svn')
547 def test_merge_pull_request_not_approved(self, pr_util, csrf_token):
549 def test_merge_pull_request_not_approved(self, pr_util, csrf_token):
@@ -556,10 +558,11 b' class TestPullrequestsController:'
556 pull_request_id=str(pull_request_id)),
558 pull_request_id=str(pull_request_id)),
557 params={'csrf_token': csrf_token}).follow()
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
563 response.mustcontain(
562 assert ' Reviewer approval is pending.' in response.body
564 'Merge is not currently possible because of below failed checks.')
565 response.mustcontain('Pull request reviewer approval is pending.')
563
566
564 def test_update_source_revision(self, backend, csrf_token):
567 def test_update_source_revision(self, backend, csrf_token):
565 commits = [
568 commits = [
General Comments 0
You need to be logged in to leave comments. Login now