diff --git a/rhodecode/controllers/pullrequests.py b/rhodecode/controllers/pullrequests.py --- a/rhodecode/controllers/pullrequests.py +++ b/rhodecode/controllers/pullrequests.py @@ -633,6 +633,18 @@ class PullrequestsController(BaseRepoCon msg = _('Pull request reviewer approval is pending.') h.flash(msg, category='error') return False + + todos = CommentsModel().get_unresolved_todos(pull_request) + if todos: + log.debug("Cannot merge, unresolved todos left.") + if len(todos) == 1: + msg = _('Cannot merge, {} todo still not resolved.').format( + len(todos)) + else: + msg = _('Cannot merge, {} todos still not resolved.').format( + len(todos)) + h.flash(msg, category='error') + return False return True def _merge_pull_request(self, pull_request, user, extras): @@ -761,6 +773,7 @@ class PullrequestsController(BaseRepoCon def show(self, repo_name, pull_request_id): pull_request_id = safe_int(pull_request_id) version = request.GET.get('version') + merge_checks = request.GET.get('merge_checks') (pull_request_latest, pull_request_at_ver, @@ -778,6 +791,10 @@ class PullrequestsController(BaseRepoCon c.shadow_clone_url = PullRequestModel().get_shadow_clone_url( pull_request_at_ver) + c.ancestor = None # TODO: add ancestor here + c.pull_request = pull_request_display_obj + c.pull_request_latest = pull_request_latest + pr_closed = pull_request_latest.is_closed() if at_version and not at_version == 'latest': c.allowed_to_change_status = False @@ -800,12 +817,6 @@ class PullrequestsController(BaseRepoCon c.pull_request_reviewers = pull_request_at_ver.reviewers_statuses() c.pull_request_review_status = pull_request_at_ver.calculated_review_status() - c.pr_merge_status, c.pr_merge_msg = PullRequestModel().merge_status( - pull_request_at_ver) - c.approval_msg = None - if c.pull_request_review_status != ChangesetStatus.STATUS_APPROVED: - c.approval_msg = _('Reviewer approval is pending.') - c.pr_merge_status = False c.versions = pull_request_display_obj.versions() c.at_version = at_version @@ -830,7 +841,7 @@ class PullrequestsController(BaseRepoCon # if we use version, then do not show later comments # than current version - paths = collections.defaultdict(lambda: collections.defaultdict(list)) + 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 @@ -842,11 +853,32 @@ class PullrequestsController(BaseRepoCon should_render = True if should_render: - paths[co.f_path][co.line_no].append(co) - inline_comments = paths + display_inline_comments[co.f_path][co.line_no].append(co) + + c.pr_merge_checks = [] + c.pr_merge_status, c.pr_merge_msg = PullRequestModel().merge_status( + pull_request_at_ver) + c.pr_merge_checks.append(['warning' if not c.pr_merge_status else 'success', c.pr_merge_msg]) + + if c.pull_request_review_status != ChangesetStatus.STATUS_APPROVED: + approval_msg = _('Reviewer approval is pending.') + c.pr_merge_status = False + c.pr_merge_checks.append(['warning', approval_msg]) + + todos = cc_model.get_unresolved_todos(pull_request_latest) + if todos: + c.pr_merge_status = False + if len(todos) == 1: + msg = _('{} todo still not resolved.').format(len(todos)) + else: + msg = _('{} todos still not resolved.').format(len(todos)) + c.pr_merge_checks.append(['warning', msg]) + + if merge_checks: + return render('/pullrequests/pullrequest_merge_checks.mako') # load compare data into template context - self._load_compare_data(pull_request_at_ver, inline_comments) + self._load_compare_data(pull_request_at_ver, display_inline_comments) # this is a hack to properly display links, when creating PR, the # compare view and others uses different notation, and @@ -861,10 +893,6 @@ class PullrequestsController(BaseRepoCon statuses = ChangesetStatus.STATUSES c.commit_statuses = statuses - c.ancestor = None # TODO: add ancestor here - c.pull_request = pull_request_display_obj - c.pull_request_latest = pull_request_latest - c.changes = None c.file_changes = None diff --git a/rhodecode/model/comment.py b/rhodecode/model/comment.py --- a/rhodecode/model/comment.py +++ b/rhodecode/model/comment.py @@ -129,6 +129,18 @@ class CommentsModel(BaseModel): return comment_versions + def get_unresolved_todos(self, pull_request): + + todos = Session().query(ChangesetComment) \ + .filter(ChangesetComment.pull_request == pull_request) \ + .filter(ChangesetComment.resolved_by == None) \ + .filter(ChangesetComment.comment_type + == ChangesetComment.COMMENT_TYPE_TODO) \ + .filter(coalesce(ChangesetComment.display_state, '') != + ChangesetComment.COMMENT_OUTDATED).all() + + return todos + def create(self, text, repo, user, commit_id=None, pull_request=None, f_path=None, line_no=None, status_change=None, status_change_type=None, comment_type=None, diff --git a/rhodecode/model/db.py b/rhodecode/model/db.py --- a/rhodecode/model/db.py +++ b/rhodecode/model/db.py @@ -2972,6 +2972,10 @@ class ChangesetComment(Base, BaseModel): def resolved(self): return self.resolved_by[0] if self.resolved_by else None + @property + def is_todo(self): + return self.comment_type == self.COMMENT_TYPE_TODO + def get_index_version(self, versions): return self.get_index_from_version( self.pull_request_version_id, versions) 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 @@ -1610,19 +1610,53 @@ BIN_FILENODE = 7 } .pull-request-merge { - padding: 10px 0; + border: 1px solid @grey5; + padding: 10px 0px 20px; margin-top: 10px; margin-bottom: 20px; } +.pull-request-merge ul { + padding: 0px 0px; +} + +.pull-request-merge li:before{ + content:none; +} + .pull-request-merge .pull-request-wrap { - height: 25px; - padding: 5px 0; + height: auto; + padding: 0px 0px; + text-align: right; } .pull-request-merge span { - margin-right: 10px; + margin-right: 5px; +} + +.pull-request-merge-actions { + height: 30px; + padding: 0px 0px; +} + +.merge-message { + font-size: 1.2em } +.merge-message li{ + text-decoration: none; +} + +.merge-message.success i { + color:@alert1; +} +.merge-message.warning i { + color: @alert3; +} +.merge-message.error i { + color:@alert2; +} + + .pr-versions { position: relative; 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 @@ -240,7 +240,12 @@ var bindToggleButtons = function() { $(this.statusChange).select2('readonly', false); }; - this.globalSubmitSuccessCallback = function(){}; + this.globalSubmitSuccessCallback = function(){ + // default behaviour is to call GLOBAL hook, if it's registered. + if (window.commentFormGlobalSubmitSuccessCallback !== undefined){ + commentFormGlobalSubmitSuccessCallback() + } + }; this.submitAjaxPOST = function(url, postData, successHandler, failHandler) { failHandler = failHandler || function() {}; @@ -300,7 +305,8 @@ var bindToggleButtons = function() { } var submitSuccessCallback = function(o) { - if (status) { + // reload page if we change status for single commit. + if (status && self.commitId) { location.reload(true); } else { $('#injected_page_comments').append(o.rendered_text); diff --git a/rhodecode/templates/changeset/changeset_file_comment.mako b/rhodecode/templates/changeset/changeset_file_comment.mako --- a/rhodecode/templates/changeset/changeset_file_comment.mako +++ b/rhodecode/templates/changeset/changeset_file_comment.mako @@ -170,29 +170,6 @@ <%def name="comments(post_url, cur_status, is_pull_request=False, is_compare=False, change_status=True, form_extras=None)"> -## merge status, and merge action -%if is_pull_request: -
- %if c.allowed_to_merge: -
-
- ${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')} - ${c.pr_merge_msg} ${c.approval_msg if c.approval_msg else ''} - <% merge_disabled = ' disabled' if c.pr_merge_status is False else '' %> - - ${h.end_form()} -
-
- %else: -
-
- ${c.pr_merge_msg} ${c.approval_msg if c.approval_msg else ''} -
-
- %endif -
-%endif -
<% if is_pull_request: diff --git a/rhodecode/templates/pullrequests/pullrequest_merge_checks.mako b/rhodecode/templates/pullrequests/pullrequest_merge_checks.mako new file mode 100644 --- /dev/null +++ b/rhodecode/templates/pullrequests/pullrequest_merge_checks.mako @@ -0,0 +1,37 @@ + +
+ + + +
+ % if c.allowed_to_merge: +
+ ${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')} + <% merge_disabled = ' disabled' if c.pr_merge_status is False else '' %> + ${_('refresh checks')} + + ${h.end_form()} +
+ % elif c.rhodecode_user.username != h.DEFAULT_USER: + ${_('refresh checks')} + + % else: + + % endif +
+ +
+ 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 @@ -482,23 +482,28 @@ Changed files:
+ % if general_outdated_comm_count_ver:
- % if general_outdated_comm_count_ver: - % if general_outdated_comm_count_ver == 1: - ${_('there is {num} general comment from older versions').format(num=general_outdated_comm_count_ver)}, - ${_('show it')} - % else: - ${_('there are {num} general comments from older versions').format(num=general_outdated_comm_count_ver)}, - ${_('show them')} - % endif + % if general_outdated_comm_count_ver == 1: + ${_('there is {num} general comment from older versions').format(num=general_outdated_comm_count_ver)}, + ${_('show it')} + % else: + ${_('there are {num} general comments from older versions').format(num=general_outdated_comm_count_ver)}, + ${_('show them')} % endif
+ % endif
${comment.generate_comments(c.comments, include_pull_request=True, is_pull_request=True)} % if not c.pull_request.is_closed(): + ## merge status, and merge action +
+ <%include file="/pullrequests/pullrequest_merge_checks.mako"/> +
+ ## 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), @@ -603,6 +608,16 @@ Changed files: $('.showOutdatedComments').show(); }; + refreshMergeChecks = function(){ + var loadUrl = "${h.url.current(merge_checks=1)}"; + $('.pull-request-merge').css('opacity', 0.3); + $('.pull-request-merge').load( + loadUrl,function() { + $('.pull-request-merge').css('opacity', 1); + } + ); + }; + $('#show-outdated-comments').on('click', function(e){ var button = $(this); var outdated = $('.comment-outdated'); @@ -683,6 +698,12 @@ Changed files: button.addClass("comments-visible"); } }); + + // register submit callback on commentForm form to track TODOs + window.commentFormGlobalSubmitSuccessCallback = function(){ + refreshMergeChecks(); + }; + })