pull-request: introduced new merge-checks.

- merge checks show nicer UI for status of merge approval inside PR
- merge checks now also forbid a merge if TODO notes are present
- use global refresh merge check when submitting comments. It
  no longer required page reload when submitting status change.

rhodecode/templates/pullrequests/pullrequest_merge_checks.mako

@@ -0,0 +1,37 b''
+
+<div class="pull-request-wrap">
+
+    <ul>
+        % for pr_check_type, pr_check_msg in c.pr_merge_checks:
+            <li>
+                <span class="merge-message ${pr_check_type}" data-role="merge-message">
+                    % if pr_check_type in ['success']:
+                        <i class="icon-true"></i>
+                    % else:
+                        <i class="icon-false"></i>
+                    % endif
+                    ${pr_check_msg}
+                </span>
+            </li>
+        % endfor
+    </ul>
+
+    <div class="pull-request-merge-actions">
+        % if c.allowed_to_merge:
+        <div class="pull-right">
+          ${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 '' %>
+          <a class="btn" href="#" onclick="refreshMergeChecks(); return false;">${_('refresh checks')}</a>
+          <input type="submit" id="merge_pull_request" value="${_('Merge Pull Request')}" class="btn${merge_disabled}"${merge_disabled}>
+          ${h.end_form()}
+        </div>
+        % elif c.rhodecode_user.username != h.DEFAULT_USER:
+            <a class="btn" href="#" onclick="refreshMergeChecks(); return false;">${_('refresh checks')}</a>
+            <input type="submit" value="${_('Merge Pull Request')}" class="btn disabled" disabled="disabled" title="${_('You are not allowed to merge this pull request.')}">
+        % else:
+          <input type="submit" value="${_('Login to Merge this Pull Request')}" class="btn disabled" disabled="disabled">
+        % endif
+    </div>
+
+</div>
+

rhodecode/controllers/pullrequests.py

@@ -633,6 +633,18 b' 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 b' 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 b' 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 b' 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 b' 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 b' 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 b' 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
 

rhodecode/model/comment.py

@@ -129,6 +129,18 b' 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,

rhodecode/model/db.py

@@ -2972,6 +2972,10 b' 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)

rhodecode/public/css/main.less

@@ -1610,19 +1610,53 b' 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;

rhodecode/public/js/src/rhodecode/comments.js

@@ -240,7 +240,12 b' 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 b' 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);

rhodecode/templates/changeset/changeset_file_comment.mako

@@ -170,29 +170,6 b''
 
 <%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:
-<div class="pull-request-merge">
-    %if c.allowed_to_merge:
-    <div class="pull-request-wrap">
-        <div class="pull-right">
-          ${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')}
-          <span data-role="merge-message">${c.pr_merge_msg} ${c.approval_msg if c.approval_msg else ''}</span>
-          <% merge_disabled = ' disabled' if c.pr_merge_status is False else '' %>
-          <input type="submit" id="merge_pull_request" value="${_('Merge Pull Request')}" class="btn${merge_disabled}"${merge_disabled}>
-          ${h.end_form()}
-        </div>
-    </div>
-    %else:
-    <div class="pull-request-wrap">
-        <div class="pull-right">
-          <span>${c.pr_merge_msg} ${c.approval_msg if c.approval_msg else ''}</span>
-        </div>
-    </div>
-    %endif
-</div>
-%endif
-
 <div class="comments">
     <%
       if is_pull_request:

rhodecode/templates/pullrequests/pullrequest_show.mako

@@ -482,8 +482,8 b' Changed files:'
 
       <div id="comment-tr-show">
           <div class="comment">
+            % if general_outdated_comm_count_ver:
             <div class="meta">
-                % 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)},
                         <a href="#" onclick="$('.comment-general.comment-outdated').show(); $(this).parent().hide(); return false;">${_('show it')}</a>
@@ -491,14 +491,19 b' Changed files:'
                         ${_('there are {num} general comments from older versions').format(num=general_outdated_comm_count_ver)},
                         <a href="#" onclick="$('.comment-general.comment-outdated').show(); $(this).parent().hide(); return false;">${_('show them')}</a>
                     % endif
+            </div>
                 % endif
             </div>
           </div>
-      </div>
 
       ${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
+        <div class="pull-request-merge">
+            <%include file="/pullrequests/pullrequest_merge_checks.mako"/>
+        </div>
+
         ## 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 b' 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 b' Changed files:'
                   button.addClass("comments-visible");
                 }
             });
+
+            // register submit callback on commentForm form to track TODOs
+            window.commentFormGlobalSubmitSuccessCallback = function(){
+                refreshMergeChecks();
+            };
+
         })
       </script>