pull-requests: added version browsing for pull requests.

- invalidated comments now link to previous version to show code context
- removed all deprecated JS code left from new diffs
- improved invalidated comments display logic
- UI tweaks

rhodecode/controllers/pullrequests.py

@@ -45,16 +45,17 b' from rhodecode.lib.auth import ('
 from rhodecode.lib.channelstream import channelstream_request
 from rhodecode.lib.compat import OrderedDict
 from rhodecode.lib.utils import jsonify
-from rhodecode.lib.utils2 import safe_int, safe_str, str2bool, safe_unicode
+from rhodecode.lib.utils2 import (
+    safe_int, safe_str, str2bool, safe_unicode, UnsafeAttributeDict)
 from rhodecode.lib.vcs.backends.base import EmptyCommit, UpdateFailureReason
 from rhodecode.lib.vcs.exceptions import (
     EmptyRepositoryError, CommitDoesNotExistError, RepositoryRequirementError,
     NodeDoesNotExistError)
-from rhodecode.lib.diffs import LimitedDiffContainer
+
 from rhodecode.model.changeset_status import ChangesetStatusModel
 from rhodecode.model.comment import ChangesetCommentsModel
-from rhodecode.model.db import PullRequest, ChangesetStatus, ChangesetComment, \
-    Repository
+from rhodecode.model.db import (PullRequest, ChangesetStatus, ChangesetComment,
+    Repository, PullRequestVersion)
 from rhodecode.model.forms import PullRequestForm
 from rhodecode.model.meta import Session
 from rhodecode.model.pull_request import PullRequestModel
@@ -675,46 +676,133 b' class PullrequestsController(BaseRepoCon'
             return redirect(url('my_account_pullrequests'))
         raise HTTPForbidden()
 
+    def _get_pr_version(self, pull_request_id, version=None):
+        pull_request_id = safe_int(pull_request_id)
+        at_version = None
+        if version:
+            pull_request_ver = PullRequestVersion.get_or_404(version)
+            pull_request_obj = pull_request_ver
+            _org_pull_request_obj = pull_request_ver.pull_request
+            at_version = pull_request_ver.pull_request_version_id
+        else:
+            _org_pull_request_obj = pull_request_obj = PullRequest.get_or_404(pull_request_id)
+
+        class PullRequestDisplay(object):
+            """
+            Special object wrapper for showing PullRequest data via Versions
+            It mimics PR object as close as possible. This is read only object
+            just for display
+            """
+            def __init__(self, attrs):
+                self.attrs = attrs
+                # internal have priority over the given ones via attrs
+                self.internal = ['versions']
+
+            def __getattr__(self, item):
+                if item in self.internal:
+                    return getattr(self, item)
+                try:
+                    return self.attrs[item]
+                except KeyError:
+                    raise AttributeError(
+                        '%s object has no attribute %s' % (self, item))
+
+            def versions(self):
+                return pull_request_obj.versions.order_by(
+                    PullRequestVersion.pull_request_version_id).all()
+
+            def is_closed(self):
+                return pull_request_obj.is_closed()
+
+        attrs = UnsafeAttributeDict(pull_request_obj.get_api_data())
+
+        attrs.author = UnsafeAttributeDict(
+            pull_request_obj.author.get_api_data())
+        if pull_request_obj.target_repo:
+            attrs.target_repo = UnsafeAttributeDict(
+                pull_request_obj.target_repo.get_api_data())
+            attrs.target_repo.clone_url = pull_request_obj.target_repo.clone_url
+
+        if pull_request_obj.source_repo:
+            attrs.source_repo = UnsafeAttributeDict(
+                pull_request_obj.source_repo.get_api_data())
+            attrs.source_repo.clone_url = pull_request_obj.source_repo.clone_url
+
+        attrs.source_ref_parts = pull_request_obj.source_ref_parts
+        attrs.target_ref_parts = pull_request_obj.target_ref_parts
+
+        attrs.shadow_merge_ref = _org_pull_request_obj.shadow_merge_ref
+
+        pull_request_ver = PullRequestDisplay(attrs)
+
+        return _org_pull_request_obj, pull_request_obj, \
+               pull_request_ver, at_version
+
     @LoginRequired()
     @HasRepoPermissionAnyDecorator('repository.read', 'repository.write',
                                    'repository.admin')
     def show(self, repo_name, pull_request_id):
         pull_request_id = safe_int(pull_request_id)
-        c.pull_request = PullRequest.get_or_404(pull_request_id)
+
+        version = request.GET.get('version')
+        pull_request_latest, \
+        pull_request, \
+        pull_request_ver, \
+        at_version = self._get_pr_version(pull_request_id, version=version)
 
         c.template_context['pull_request_data']['pull_request_id'] = \
             pull_request_id
 
         # pull_requests repo_name we opened it against
         # ie. target_repo must match
-        if repo_name != c.pull_request.target_repo.repo_name:
+        if repo_name != pull_request.target_repo.repo_name:
             raise HTTPNotFound
 
+        c.shadow_clone_url = PullRequestModel().get_shadow_clone_url(
+            pull_request)
+
+        if at_version:
+            c.allowed_to_change_status = False
+        else:
         c.allowed_to_change_status = PullRequestModel(). \
-            check_user_change_status(c.pull_request, c.rhodecode_user)
+                check_user_change_status(pull_request, c.rhodecode_user)
+
+        if at_version:
+            c.allowed_to_update = False
+        else:
         c.allowed_to_update = PullRequestModel().check_user_update(
-            c.pull_request, c.rhodecode_user) and not c.pull_request.is_closed()
+                pull_request, c.rhodecode_user) and not pull_request.is_closed()
+
+        if at_version:
+            c.allowed_to_merge = False
+        else:
         c.allowed_to_merge = PullRequestModel().check_user_merge(
-            c.pull_request, c.rhodecode_user) and not c.pull_request.is_closed()
-        c.shadow_clone_url = PullRequestModel().get_shadow_clone_url(
-            c.pull_request)
+                pull_request, c.rhodecode_user) and not pull_request.is_closed()
+
+        if at_version:
+            c.allowed_to_delete = False
+        else:
         c.allowed_to_delete = PullRequestModel().check_user_delete(
-            c.pull_request, c.rhodecode_user) and not c.pull_request.is_closed()
+                pull_request, c.rhodecode_user) and not pull_request.is_closed()
+
+        if at_version:
+            c.allowed_to_comment = False
+        else:
+            c.allowed_to_comment = not pull_request.is_closed()
 
         cc_model = ChangesetCommentsModel()
 
-        c.pull_request_reviewers = c.pull_request.reviewers_statuses()
+        c.pull_request_reviewers = pull_request.reviewers_statuses()
 
-        c.pull_request_review_status = c.pull_request.calculated_review_status()
+        c.pull_request_review_status = pull_request.calculated_review_status()
         c.pr_merge_status, c.pr_merge_msg = PullRequestModel().merge_status(
-            c.pull_request)
+            pull_request)
         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
         # load compare data into template context
-        enable_comments = not c.pull_request.is_closed()
-
+        enable_comments = not pull_request.is_closed()
 
         # inline comments
         c.inline_comments = cc_model.get_inline_comments(
@@ -725,23 +813,26 b' class PullrequestsController(BaseRepoCon'
             c.inline_comments, version=at_version)
 
         self._load_compare_data(
-            c.pull_request, c.inline_comments, enable_comments=enable_comments)
+            pull_request, c.inline_comments, enable_comments=enable_comments)
 
         # outdated comments
         c.outdated_comments = {}
         c.outdated_cnt = 0
-        if ChangesetCommentsModel.use_outdated_comments(c.pull_request):
+
+        if ChangesetCommentsModel.use_outdated_comments(pull_request):
             c.outdated_comments = cc_model.get_outdated_comments(
                 c.rhodecode_db_repo.repo_id,
-                pull_request=c.pull_request)
+                pull_request=pull_request)
+
             # Count outdated comments and check for deleted files
             for file_name, lines in c.outdated_comments.iteritems():
                 for comments in lines.values():
+                    comments = [comm for comm in comments
+                                if comm.outdated_at_version(at_version)]
                     c.outdated_cnt += len(comments)
                 if file_name not in c.included_files:
                     c.deleted_files.append(file_name)
 
-
         # this is a hack to properly display links, when creating PR, the
         # compare view and others uses different notation, and
         # compare_commits.html renders links based on the target_repo.
@@ -760,6 +851,9 b' class PullrequestsController(BaseRepoCon'
         c.commit_statuses = statuses
 
         c.ancestor = None # TODO: add ancestor here
+        c.pull_request = pull_request_ver
+        c.pull_request_latest = pull_request_latest
+        c.at_version = at_version
 
         return render('/pullrequests/pullrequest_show.html')
 
@@ -813,8 +907,6 b' class PullrequestsController(BaseRepoCon'
             closing_pr=close_pr
         )
 
-
-
         if allowed_to_change_status:
             old_calculated_status = pull_request.calculated_review_status()
             # get status if set !

rhodecode/lib/utils2.py

@@ -656,6 +656,16 b' def extract_mentioned_users(s):'
     return sorted(list(usrs), key=lambda k: k.lower())
 
 
+class UnsafeAttributeDict(dict):
+    def __getattr__(self, attr):
+        try:
+            return self[attr]
+        except KeyError:
+            raise AttributeError('%s object has no attribute %s' % (self, attr))
+    __setattr__ = dict.__setitem__
+    __delattr__ = dict.__delitem__
+
+
 class AttributeDict(dict):
     def __getattr__(self, attr):
         return self.get(attr, None)

rhodecode/model/__init__.py

@@ -90,7 +90,7 b' class BaseModel(object):'
         """
         Gets instance of given cls using some simple lookup mechanism.
 
-        :param cls: class to fetch
+        :param cls: classes to fetch
         :param instance: int or Instance
         :param callback: callback to call if all lookups failed
         """
@@ -98,6 +98,9 b' class BaseModel(object):'
         if isinstance(instance, cls):
             return instance
         elif isinstance(instance, (int, long)):
+            if isinstance(cls, tuple):
+                # if we pass multi instances we pick first to .get()
+                cls = cls[0]
             return cls.get(instance)
         else:
             if instance:

rhodecode/model/db.py

@@ -2933,6 +2933,12 b' class ChangesetComment(Base, BaseModel):'
     def outdated(self):
         return self.display_state == self.COMMENT_OUTDATED
 
+    def outdated_at_version(self, version):
+        """
+        Checks if comment is outdated for given pull request version
+        """
+        return self.outdated and self.pull_request_version_id != version
+
     def render(self, mentions=False):
         from rhodecode.lib import helpers as h
         return h.render(self.text, renderer=self.renderer, mentions=mentions)
@@ -3117,34 +3123,6 b' class _PullRequestBase(BaseModel):'
         else:
             return None
 
-
-class PullRequest(Base, _PullRequestBase):
-    __tablename__ = 'pull_requests'
-    __table_args__ = (
-        {'extend_existing': True, 'mysql_engine': 'InnoDB',
-         'mysql_charset': 'utf8', 'sqlite_autoincrement': True},
-    )
-
-    pull_request_id = Column(
-        'pull_request_id', Integer(), nullable=False, primary_key=True)
-
-    def __repr__(self):
-        if self.pull_request_id:
-            return '<DB:PullRequest #%s>' % self.pull_request_id
-        else:
-            return '<DB:PullRequest at %#x>' % id(self)
-
-    reviewers = relationship('PullRequestReviewers',
-                             cascade="all, delete, delete-orphan")
-    statuses = relationship('ChangesetStatus')
-    comments = relationship('ChangesetComment',
-                            cascade="all, delete, delete-orphan")
-    versions = relationship('PullRequestVersion',
-                            cascade="all, delete, delete-orphan")
-
-    def is_closed(self):
-        return self.status == self.STATUS_CLOSED
-
     def get_api_data(self):
         from rhodecode.model.pull_request import PullRequestModel
         pull_request = self
@@ -3209,6 +3187,35 b' class PullRequest(Base, _PullRequestBase'
 
         return data
 
+
+class PullRequest(Base, _PullRequestBase):
+    __tablename__ = 'pull_requests'
+    __table_args__ = (
+        {'extend_existing': True, 'mysql_engine': 'InnoDB',
+         'mysql_charset': 'utf8', 'sqlite_autoincrement': True},
+    )
+
+    pull_request_id = Column(
+        'pull_request_id', Integer(), nullable=False, primary_key=True)
+
+    def __repr__(self):
+        if self.pull_request_id:
+            return '<DB:PullRequest #%s>' % self.pull_request_id
+        else:
+            return '<DB:PullRequest at %#x>' % id(self)
+
+    reviewers = relationship('PullRequestReviewers',
+                             cascade="all, delete, delete-orphan")
+    statuses = relationship('ChangesetStatus')
+    comments = relationship('ChangesetComment',
+                            cascade="all, delete, delete-orphan")
+    versions = relationship('PullRequestVersion',
+                            cascade="all, delete, delete-orphan",
+                            lazy='dynamic')
+
+    def is_closed(self):
+        return self.status == self.STATUS_CLOSED
+
     def __json__(self):
         return {
             'revisions': self.revisions,
@@ -3243,6 +3250,24 b' class PullRequestVersion(Base, _PullRequ'
         else:
             return '<DB:PullRequestVersion at %#x>' % id(self)
 
+    @property
+    def reviewers(self):
+        return self.pull_request.reviewers
+
+    @property
+    def versions(self):
+        return self.pull_request.versions
+
+    def is_closed(self):
+        # calculate from original
+        return self.pull_request.status == self.STATUS_CLOSED
+
+    def calculated_review_status(self):
+        return self.pull_request.calculated_review_status()
+
+    def reviewers_statuses(self):
+        return self.pull_request.reviewers_statuses()
+
 
 class PullRequestReviewers(Base, BaseModel):
     __tablename__ = 'pull_request_reviewers'

rhodecode/public/css/buttons.less

@@ -208,8 +208,14 b' input[type="button"] {'
         color: @rcdarkblue;
     }
 
+    //disabled buttons
+    //last; overrides any other styles
     &:disabled {
+        opacity: .7;
+        cursor: auto;
+        background-color: white;
         color: @grey4;
+        text-shadow: none;
     }
 
     // TODO: johbo: Check if we can avoid this, indicates that the structure

rhodecode/public/css/code-block.less

@@ -313,7 +313,7 b' table.code-difftable {'
 // Comments
 
 div.comment:target {
-    border-left: 6px solid @comment-highlight-color;
+    border-left: 6px solid @comment-highlight-color !important;
     padding-left: 3px;
     margin-left: -9px;
 }
@@ -737,6 +737,15 b' input.filediff-collapse-state {'
     }
 }
 
+/* Main comments*/
+#comments {
+    .comment-selected {
+        border-left: 6px solid @comment-highlight-color;
+        padding-left: 3px;
+        margin-left: -9px;
+    }
+}
+
 .filediff {
     border: 1px solid @grey5;
 
@@ -894,6 +903,7 b' input.filediff-collapse-state {'
             display: none;
         }
     }
+
     .inline-comments {
         border-radius: @border-radius;
         background: @grey6;
@@ -904,6 +914,7 b' input.filediff-collapse-state {'
         .comment-outdated {
             opacity: 0.5;
         }
+
         .comment-inline {
             background: white;
             padding: (@comment-padding + 3px) @comment-padding;

rhodecode/public/css/comments.less

@@ -170,22 +170,6 b' tr.inline-comments div {'
     color: @rcblue;
 }
 
-.outdated {
-    display: none;
-    opacity: 0.6;
-
-    .comment {
-        margin: 0 0 @padding;
-
-        .date:after {
-            content: none;
-        }
-    }
-    .outdated_comment_block {
-        padding: 0 0 @space 0;
-    }
-}
-
 // Comment Form
 div.comment-form {
     margin-top: 20px;

rhodecode/public/css/main.less

@@ -1395,9 +1395,7 b' table.integrations {'
     width: 92%;
     margin-bottom: 1em;
 }
-#update_commits {
-    float: right;
-}
+
 .compare_view_commits tr{
     height: 20px;
 }

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

@@ -48,58 +48,6 b' var tableTr = function(cls, body){'
   return _el.children[0].children[0].children[0];
 };
 
-var removeInlineForm = function(form) {
-  form.parentNode.removeChild(form);
-};
-
-var createInlineForm = function(parent_tr, f_path, line) {
-  var tmpl = $('#comment-inline-form-template').html();
-  tmpl = tmpl.format(f_path, line);
-  var form = tableTr('comment-form-inline', tmpl);
-  var form_hide_button = $(form).find('.hide-inline-form');
-
-  $(form_hide_button).click(function(e) {
-     $('.inline-comments').removeClass('hide-comment-button');
-     var newtr = e.currentTarget.parentNode.parentNode.parentNode.parentNode.parentNode;
-     if ($(newtr.nextElementSibling).hasClass('inline-comments-button')) {
-         $(newtr.nextElementSibling).show();
-     }
-     $(newtr).parents('.comment-form-inline').remove();
-     $(parent_tr).removeClass('form-open');
-     $(parent_tr).removeClass('hl-comment');
-  });
-
-  return form;
-};
-
-var getLineNo = function(tr) {
-  var line;
-  // Try to get the id and return "" (empty string) if it doesn't exist
-  var o = ($(tr).find('.lineno.old').attr('id')||"").split('_');
-  var n = ($(tr).find('.lineno.new').attr('id')||"").split('_');
-  if (n.length >= 2) {
-    line = n[n.length-1];
-  } else if (o.length >= 2) {
-   line = o[o.length-1];
-  }
-  return line;
-};
-
-/**
- * make a single inline comment and place it inside
- */
-var renderInlineComment = function(json_data, show_add_button) {
-  show_add_button = typeof show_add_button !== 'undefined' ? show_add_button : true;
-  try {
-    var html = json_data.rendered_text;
-    var lineno = json_data.line_no;
-    var target_id = json_data.target_id;
-    placeInline(target_id, lineno, html, show_add_button);
-  } catch (e) {
-    console.error(e);
-  }
-};
-
 function bindDeleteCommentButtons() {
     $('.delete-comment').one('click', function() {
         var comment_id = $(this).data("comment-id");
@@ -110,115 +58,6 b' function bindDeleteCommentButtons() {'
     });
 }
 
-/**
- * Inject inline comment for on given TR this tr should be always an .line
- * tr containing the line. Code will detect comment, and always put the comment
- * block at the very bottom
- */
-var injectInlineForm = function(tr){
-  if (!$(tr).hasClass('line')) {
-      return;
-  }
-
-  var _td = $(tr).find('.code').get(0);
-  if ($(tr).hasClass('form-open') ||
-      $(tr).hasClass('context') ||
-      $(_td).hasClass('no-comment')) {
-      return;
-  }
-  $(tr).addClass('form-open');
-  $(tr).addClass('hl-comment');
-  var node = $(tr.parentNode.parentNode.parentNode).find('.full_f_path').get(0);
-  var f_path = $(node).attr('path');
-  var lineno = getLineNo(tr);
-  var form = createInlineForm(tr, f_path, lineno);
-
-  var parent = tr;
-  while (1) {
-      var n = parent.nextElementSibling;
-      // next element are comments !
-      if ($(n).hasClass('inline-comments')) {
-          parent = n;
-      }
-      else {
-          break;
-      }
-  }
-  var _parent = $(parent).get(0);
-  $(_parent).after(form);
-  $('.comment-form-inline').prev('.inline-comments').addClass('hide-comment-button');
-  var f = $(form).get(0);
-
-  var _form = $(f).find('.inline-form').get(0);
-
-  var pullRequestId = templateContext.pull_request_data.pull_request_id;
-  var commitId = templateContext.commit_data.commit_id;
-
-  var commentForm = new CommentForm(_form, commitId, pullRequestId, lineno, false);
-  var cm = commentForm.getCmInstance();
-
-  // set a CUSTOM submit handler for inline comments.
-  commentForm.setHandleFormSubmit(function(o) {
-    var text = commentForm.cm.getValue();
-
-    if (text === "") {
-      return;
-    }
-
-    if (lineno === undefined) {
-      alert('missing line !');
-      return;
-    }
-    if (f_path === undefined) {
-      alert('missing file path !');
-      return;
-    }
-
-    var excludeCancelBtn = false;
-    var submitEvent = true;
-    commentForm.setActionButtonsDisabled(true, excludeCancelBtn, submitEvent);
-    commentForm.cm.setOption("readOnly", true);
-    var postData = {
-        'text': text,
-        'f_path': f_path,
-        'line': lineno,
-        'csrf_token': CSRF_TOKEN
-    };
-    var submitSuccessCallback = function(o) {
-      $(tr).removeClass('form-open');
-      removeInlineForm(f);
-      renderInlineComment(o);
-      $('.inline-comments').removeClass('hide-comment-button');
-
-      // re trigger the linkification of next/prev navigation
-      linkifyComments($('.inline-comment-injected'));
-      timeagoActivate();
-      bindDeleteCommentButtons();
-      commentForm.setActionButtonsDisabled(false);
-
-    };
-    var submitFailCallback = function(){
-        commentForm.resetCommentFormState(text)
-    };
-    commentForm.submitAjaxPOST(
-        commentForm.submitUrl, postData, submitSuccessCallback, submitFailCallback);
-  });
-
-  setTimeout(function() {
-      // callbacks
-      if (cm !== undefined) {
-          cm.focus();
-      }
-  }, 10);
-
-    $.Topic('/ui/plugins/code/comment_form_built').prepareOrPublish({
-        form:_form,
-        parent:_parent,
-        lineno: lineno,
-        f_path: f_path}
-    );
-};
-
 var deleteComment = function(comment_id) {
   var url = AJAX_COMMENT_DELETE_URL.replace('__COMMENT_ID__', comment_id);
   var postData = {
@@ -232,89 +71,6 b' var deleteComment = function(comment_id)'
   ajaxPOST(url, postData, success);
 };
 
-var createInlineAddButton = function(tr){
-  var label = _gettext('Add another comment');
-  var html_el = document.createElement('div');
-  $(html_el).addClass('add-comment');
-  html_el.innerHTML = '<span class="btn btn-secondary">{0}</span>'.format(label);
-  var add = new $(html_el);
-  add.on('click', function(e) {
-    injectInlineForm(tr);
-  });
-  return add;
-};
-
-var placeAddButton = function(target_tr){
-  if(!target_tr){
-    return;
-  }
-  var last_node = target_tr;
-  // scan
-  while (1){
-    var n = last_node.nextElementSibling;
-    // next element are comments !
-    if($(n).hasClass('inline-comments')){
-      last_node = n;
-      // also remove the comment button from previous
-      var comment_add_buttons = $(last_node).find('.add-comment');
-      for(var i=0; i<comment_add_buttons.length; i++){
-        var b = comment_add_buttons[i];
-        b.parentNode.removeChild(b);
-      }
-    }
-    else{
-      break;
-    }
-  }
-  var add = createInlineAddButton(target_tr);
-  // get the comment div
-  var comment_block = $(last_node).find('.comment')[0];
-  // attach add button
-  $(add).insertAfter(comment_block);
-};
-
-/**
- * Places the inline comment into the changeset block in proper line position
- */
-var placeInline = function(target_container, lineno, html, show_add_button) {
-  show_add_button = typeof show_add_button !== 'undefined' ? show_add_button : true;
-
-  var lineid = "{0}_{1}".format(target_container, lineno);
-  var target_line = $('#' + lineid).get(0);
-  var comment = new $(tableTr('inline-comments', html));
-  // check if there are comments already !
-  if (target_line) {
-    var parent_node = target_line.parentNode;
-    var root_parent = parent_node;
-
-    while (1) {
-      var n = parent_node.nextElementSibling;
-      // next element are comments !
-      if ($(n).hasClass('inline-comments')) {
-        parent_node = n;
-      }
-      else {
-        break;
-      }
-    }
-    // put in the comment at the bottom
-    $(comment).insertAfter(parent_node);
-    $(comment).find('.comment-inline').addClass('inline-comment-injected');
-    // scan nodes, and attach add button to last one
-    if (show_add_button) {
-      placeAddButton(root_parent);
-    }
-    addCommentToggle(target_line);
-  }
-
-  return target_line;
-};
-
-var addCommentToggle = function(target_line) {
-  // exposes comment toggle button
-  $(target_line).siblings('.comment-toggle').addClass('active');
-  return;
-};
 
 var bindToggleButtons = function() {
   $('.comment-toggle').on('click', function() {
@@ -348,37 +104,6 b' var linkifyComments = function(comments)'
 
 };
 
-/**
- * Iterates over all the inlines, and places them inside proper blocks of data
- */
-var renderInlineComments = function(file_comments, show_add_button) {
-  show_add_button = typeof show_add_button !== 'undefined' ? show_add_button : true;
-
-  for (var i = 0; i < file_comments.length; i++) {
-    var box = file_comments[i];
-
-    var target_id = $(box).attr('target_id');
-
-    // actually comments with line numbers
-    var comments = box.children;
-
-    for (var j = 0; j < comments.length; j++) {
-      var data = {
-        'rendered_text': comments[j].outerHTML,
-        'line_no': $(comments[j]).attr('line'),
-        'target_id': target_id
-      };
-      renderInlineComment(data, show_add_button);
-    }
-  }
-
-  // since order of injection is random, we're now re-iterating
-  // from correct order and filling in links
-  linkifyComments($('.inline-comment-injected'));
-  bindDeleteCommentButtons();
-  firefoxAnchorFix();
-};
-
 
 /* Comment form for main and inline comments */
 var CommentForm = (function() {
@@ -679,11 +404,13 b' var CommentsController = function() { /*'
       var $td = $node.closest('td');
       $node.closest('.comment-inline-form').removeClass('comment-inline-form-open');
       return false;
-  }
+  };
+
   this.getLineNumber = function(node) {
       var $node = $(node);
       return $node.closest('td').attr('data-line-number');
-  }
+  };
+
   this.scrollToComment = function(node, offset) {
     if (!node) {
       node = $('.comment-selected');
@@ -702,20 +429,23 b' var CommentsController = function() { /*'
     }
     var $next = $('.comment-current').eq(nextIdx);
     var $cb = $next.closest('.cb');
-    $cb.removeClass('cb-collapsed')
+    $cb.removeClass('cb-collapsed');
 
     var $filediffCollapseState = $cb.closest('.filediff').prev();
     $filediffCollapseState.prop('checked', false);
     $next.addClass('comment-selected');
     scrollToElement($next);
     return false;
-  }
+  };
+
   this.nextComment = function(node) {
     return self.scrollToComment(node, 1);
-  }
+  };
+
   this.prevComment = function(node) {
     return self.scrollToComment(node, -1);
-  }
+  };
+
   this.deleteComment = function(node) {
       if (!confirm(_gettext('Delete this comment?'))) {
         return false;
@@ -744,7 +474,8 b' var CommentsController = function() { /*'
         return false;
       };
       ajaxPOST(url, postData, success, failure);
-  }
+  };
+
   this.toggleComments = function(node, show) {
     var $filediff = $(node).closest('.filediff');
     if (show === true) {
@@ -757,12 +488,14 b' var CommentsController = function() { /*'
       $filediff.toggleClass('hide-comments');
     }
     return false;
-  }
+  };
+
   this.toggleLineComments = function(node) {
     self.toggleComments(node, true);
     var $node = $(node);
     $node.closest('tr').toggleClass('hide-line-comments');
-  }
+  };
+
   this.createComment = function(node) {
       var $node = $(node);
       var $td = $node.closest('td');
@@ -832,7 +565,6 b' var CommentsController = function() { /*'
                 console.error(e);
               }
 
-
               // re trigger the linkification of next/prev navigation
               linkifyComments($('.inline-comment-injected'));
               timeagoActivate();
@@ -863,7 +595,7 b' var CommentsController = function() { /*'
       }
 
       $form.addClass('comment-inline-form-open');
-  }
+  };
 
   this.renderInlineComments = function(file_comments) {
     show_add_button = typeof show_add_button !== 'undefined' ? show_add_button : true;
@@ -892,4 +624,4 b' var CommentsController = function() { /*'
     firefoxAnchorFix();
   };
 
-}
  No newline at end of file
+};
  No newline at end of file

rhodecode/templates/changeset/changeset.html

@@ -178,9 +178,8 b''
 
     ## template for inline comment form
     <%namespace name="comment" file="/changeset/changeset_file_comment.html"/>
-    ${comment.comment_inline_form()}
 
-    ## ## render comments and inlines
+    ## render comments
     ${comment.generate_comments()}
 
     ## main comment form and it status
@@ -210,12 +209,12 b''
               if(button.hasClass("comments-visible")) {
                 $('#{0} .inline-comments'.format(boxid)).each(function(index){
                   $(this).hide();
-                })
+                });
                 button.removeClass("comments-visible");
               } else {
                 $('#{0} .inline-comments'.format(boxid)).each(function(index){
                   $(this).show();
-                })
+                });
                 button.addClass("comments-visible");
               }
           });
@@ -230,7 +229,7 b''
                     url: '${h.url('changeset_children',repo_name=c.repo_name, revision=c.commit.raw_id)}',
                     success: function(data) {
                       if(data.results.length === 0){
-                          $('#child_link').html('${_('No Child Commits')}').addClass('disabled');
+                          $('#child_link').html("${_('No Child Commits')}").addClass('disabled');
                       }
                       if(data.results.length === 1){
                           var commit = data.results[0];
@@ -263,7 +262,7 b''
               // >1 links show them to user to choose
               if(!$('#parent_link').hasClass('disabled')){
                   $.ajax({
-                    url: '${h.url('changeset_parents',repo_name=c.repo_name, revision=c.commit.raw_id)}',
+                    url: '${h.url("changeset_parents",repo_name=c.repo_name, revision=c.commit.raw_id)}',
                     success: function(data) {
                       if(data.results.length === 0){
                           $('#parent_link').html('${_('No Parent Commits')}').addClass('disabled');

rhodecode/templates/changeset/changeset_file_comment.html

@@ -6,14 +6,14 b''
 <%namespace name="base" file="/base/base.html"/>
 
 <%def name="comment_block(comment, inline=False)">
-  <div
-    class="comment
+  <div class="comment
     ${'comment-inline' if inline else ''}
-    ${'comment-outdated' if comment.outdated else 'comment-current'}"
-    "
+             ${'comment-outdated' if comment.outdated_at_version(getattr(c, 'at_version', None)) else 'comment-current'}"
     id="comment-${comment.comment_id}"
     line="${comment.line_no}"
-    data-comment-id="${comment.comment_id}">
+       data-comment-id="${comment.comment_id}"
+       style="${'display: none;' if comment.outdated_at_version(getattr(c, 'at_version', None)) else ''}">
+
       <div class="meta">
           <div class="author">
               ${base.gravatar_with_user(comment.author.email, 16)}
@@ -23,6 +23,11 b''
           </div>
           <div class="status-change">
               %if comment.pull_request:
+                  % if comment.outdated:
+                      <a href="?version=${comment.pull_request_version_id}#comment-${comment.comment_id}">
+                        ${_('Outdated comment from pull request version {}').format(comment.pull_request_version_id)}
+                      </a>
+                  % else:
                   <a href="${h.url('pullrequest_show',repo_name=comment.pull_request.target_repo.repo_name,pull_request_id=comment.pull_request.pull_request_id)}">
                       %if comment.status_change:
                           ${_('Vote on pull request #%s') % comment.pull_request.pull_request_id}:
@@ -30,6 +35,7 b''
                           ${_('Comment on pull request #%s') % comment.pull_request.pull_request_id}
                       %endif
                   </a>
+                  % endif
               %else:
                   %if comment.status_change:
                       ${_('Status change on commit')}:
@@ -52,14 +58,19 b''
             ## show delete comment if it's not a PR (regular comments) or it's PR that is not closed
             ## only super-admin, repo admin OR comment owner can delete
             %if not comment.pull_request or (comment.pull_request and not comment.pull_request.is_closed()):
+               ## permissions to delete
                %if h.HasPermissionAny('hg.admin')() or h.HasRepoPermissionAny('repository.admin')(c.repo_name) or comment.author.user_id == c.rhodecode_user.user_id:
                   ## TODO: dan: add edit comment here
-                  <a onclick="return Rhodecode.comments.deleteComment(this);" class="delete-comment"> ${_('Delete')}</a> |
-                  %if not comment.outdated:
-                  <a onclick="return Rhodecode.comments.prevComment(this);" class="prev-comment"> ${_('Prev')}</a> |
-                  <a onclick="return Rhodecode.comments.nextComment(this);" class="next-comment"> ${_('Next')}</a>
+                  <a onclick="return Rhodecode.comments.deleteComment(this);" class="delete-comment"> ${_('Delete')}</a>
+               %else:
+                  <button class="btn-link" disabled="disabled"> ${_('Delete')}</button>
                   %endif
+            %else:
+               <button class="btn-link" disabled="disabled"> ${_('Delete')}</button>
                %endif
+            %if not comment.outdated_at_version(getattr(c, 'at_version', None)):
+               | <a onclick="return Rhodecode.comments.prevComment(this);" class="prev-comment"> ${_('Prev')}</a>
+               | <a onclick="return Rhodecode.comments.nextComment(this);" class="next-comment"> ${_('Next')}</a>
             %endif
 
           </div>
@@ -67,102 +78,9 b''
       <div class="text">
           ${comment.render(mentions=True)|n}
       </div>
-  </div>
-</%def>
 
-<%def name="comment_block_outdated(comment)">
-  <div class="comments" id="comment-${comment.comment_id}">
-    <div class="comment comment-wrapp">
-      <div class="meta">
-        <div class="author">
-          ${base.gravatar_with_user(comment.author.email, 16)}
-        </div>
-        <div class="date">
-            ${h.age_component(comment.modified_at, time_is_local=True)}
-        </div>
-         %if comment.status_change:
-           <span class="changeset-status-container">
-             <span class="changeset-status-ico">
-                 <div class="${'flag_status %s' % comment.status_change[0].status}"></div>
-             </span>
-             <span title="${_('Commit status')}" class="changeset-status-lbl"> ${comment.status_change[0].status_lbl}</span>
-           </span>
-         %endif
-         <a class="permalink" href="#comment-${comment.comment_id}">&para;</a>
-         ## show delete comment if it's not a PR (regular comments) or it's PR that is not closed
-         ## only super-admin, repo admin OR comment owner can delete
-         %if not comment.pull_request or (comment.pull_request and not comment.pull_request.is_closed()):
-            <div class="comment-links-block">
-               %if h.HasPermissionAny('hg.admin')() or h.HasRepoPermissionAny('repository.admin')(c.repo_name) or comment.author.user_id == c.rhodecode_user.user_id:
-                   <div data-comment-id=${comment.comment_id} class="delete-comment">${_('Delete')}</div>
-               %endif
-            </div>
-         %endif
-      </div>
-      <div class="text">
-          ${comment.render(mentions=True)|n}
-      </div>
-    </div>
   </div>
 </%def>
-
-<%def name="comment_inline_form()">
-  <div id="comment-inline-form-template" style="display: none;">
-    <div class="comment-inline-form ac">
-    %if c.rhodecode_user.username != h.DEFAULT_USER:
-        ${h.form('#', class_='inline-form', method='get')}
-        <div id="edit-container_{1}" class="clearfix">
-          <div class="comment-title pull-left">
-            ${_('Create a comment on line {1}.')}
-          </div>
-          <div class="comment-help pull-right">
-            ${(_('Comments parsed using %s syntax with %s support.') % (
-                   ('<a href="%s">%s</a>' % (h.url('%s_help' % c.visual.default_renderer), c.visual.default_renderer.upper())),
-                     ('<span  class="tooltip" title="%s">@mention</span>' % _('Use @username inside this text to send notification to this RhodeCode user'))
-                 )
-              )|n
-             }
-          </div>
-              <div style="clear: both"></div>
-              <textarea id="text_{1}" name="text" class="comment-block-ta ac-input"></textarea>
-        </div>
-        <div id="preview-container_{1}" class="clearfix" style="display: none;">
-           <div class="comment-help">
-                ${_('Comment preview')}
-            </div>
-            <div id="preview-box_{1}" class="preview-box"></div>
-        </div>
-        <div class="comment-footer">
-          <div class="comment-button hide-inline-form-button cancel-button">
-            ${h.reset('hide-inline-form', _('Cancel'), class_='btn hide-inline-form', id_="cancel-btn_{1}")}
-          </div>
-          <div class="action-buttons">
-            <input type="hidden" name="f_path" value="{0}">
-            <input type="hidden" name="line" value="{1}">
-            <button id="preview-btn_{1}" class="btn btn-secondary">${_('Preview')}</button>
-            <button id="edit-btn_{1}" class="btn btn-secondary" style="display: none;">${_('Edit')}</button>
-            ${h.submit('save', _('Comment'), class_='btn btn-success save-inline-form')}
-          </div>
-        ${h.end_form()}
-      </div>
-    %else:
-        ${h.form('', class_='inline-form comment-form-login', method='get')}
-        <div class="pull-left">
-            <div class="comment-help pull-right">
-              ${_('You need to be logged in to comment.')} <a href="${h.route_path('login', _query={'came_from': h.url.current()})}">${_('Login now')}</a>
-            </div>
-        </div>
-        <div class="comment-button pull-right">
-        ${h.reset('hide-inline-form', _('Hide'), class_='btn hide-inline-form')}
-        </div>
-        <div class="clearfix"></div>
-        ${h.end_form()}
-    %endif
-    </div>
-  </div>
-</%def>
-
-
 ## generate main comments
 <%def name="generate_comments(include_pull_request=False, is_pull_request=False)">
   <div id="comments">
@@ -182,6 +100,7 b''
 
 ## MAIN COMMENT FORM
 <%def name="comments(post_url, cur_status, is_pull_request=False, is_compare=False, change_status=True, form_extras=None)">
+
 %if is_compare:
   <% form_id = "comments_form_compare" %>
 %else:

rhodecode/templates/codeblocks/diffs.html

@@ -394,10 +394,13 b' from rhodecode.lib.diffs import NEW_FILE'
     %for comment in comments:
     ${commentblock.comment_block(comment, inline=True)}
     %endfor
+
     <span onclick="return Rhodecode.comments.createComment(this)"
-          class="btn btn-secondary cb-comment-add-button">
+          class="btn btn-secondary cb-comment-add-button ${'comment-outdated' if comments and comments[-1].outdated else ''}"
+          style="${'display: none;' if comments and comments[-1].outdated else ''}">
         ${_('Add another comment')}
     </span>
+
 </div>
 </%def>
 

rhodecode/templates/debug_style/collapsable-content.html

@@ -918,9 +918,6 b''
                 $(btns).each(fn_display);
             });
 
-            // inject comments into they proper positions
-            var file_comments = $('.inline-comment-placeholder');
-            renderInlineComments(file_comments);
             var commentTotals = {};
             $.each(file_comments, function(i, comment) {
                 var path = $(comment).attr('path');

rhodecode/templates/pullrequests/pullrequest_show.html

@@ -165,24 +165,62 b''
                 <div>
                     <div class="comments-number">
                         %if c.comments:
-                            <a href="#comments">${ungettext("%d Pull request comment", "%d Pull request comments", len(c.comments)) % len(c.comments)}</a>,
+                            <a href="#comments">${ungettext("%d General Comment", "%d General Comments", len(c.comments)) % len(c.comments)}</a>,
                         %else:
-                            ${ungettext("%d Pull request comment", "%d Pull request comments", len(c.comments)) % len(c.comments)}
+                            ${ungettext("%d General Comment", "%d General Comments", len(c.comments)) % len(c.comments)}
                         %endif
+
                         %if c.inline_cnt:
                             <a href="#" onclick="return Rhodecode.comments.nextComment();" id="inline-comments-counter">${ungettext("%d Inline Comment", "%d Inline Comments", c.inline_cnt) % c.inline_cnt}</a>
                         %else:
                             ${ungettext("%d Inline Comment", "%d Inline Comments", c.inline_cnt) % c.inline_cnt}
                         %endif
 
-
                         % if c.outdated_cnt:
                           ,${ungettext("%d Outdated Comment", "%d Outdated Comments", c.outdated_cnt) % c.outdated_cnt} <span id="show-outdated-comments" class="btn btn-link">${_('(Show)')}</span>
                         % endif
                    </div>
                 </div>
             </div>
+
            </div>
+
+           <div class="field">
+                <div class="label-summary">
+                <label>${_('Versions')}:</label>
+           </div>
+           <div>
+               <table>
+                   <tr>
+                       <td>
+                           % if c.at_version == None:
+                            <i class="icon-ok link"></i>
+                           % endif
+                       </td>
+                       <td><code><a href="${h.url.current()}">latest</a></code></td>
+                       <td>
+                           <code>${c.pull_request_latest.source_ref_parts.commit_id[:6]}</code>
+                       </td>
+                       <td>${_('created')} ${h.age_component(c.pull_request.created_on)}</td>
+                   </tr>
+                   % for ver in reversed(c.pull_request.versions()):
+                   <tr>
+                       <td>
+                           % if c.at_version == ver.pull_request_version_id:
+                            <i class="icon-ok link"></i>
+                           % endif
+                       </td>
+                       <td><code><a href="${h.url.current(version=ver.pull_request_version_id)}">version ${ver.pull_request_version_id}</a></code></td>
+                       <td>
+                           <code>${ver.source_ref_parts.commit_id[:6]}</code>
+                       </td>
+                       <td>${_('created')} ${h.age_component(ver.created_on)}</td>
+                   </tr>
+                   % endfor
+               </table>
+           </div>
+           </div>
+
            <div id="pr-save" class="field" style="display: none;">
             <div class="label-summary"></div>
             <div class="input">
@@ -291,7 +329,9 b''
               % endif
                 <div class="compare_view_commits_title">
                     % if c.allowed_to_update and not c.pull_request.is_closed():
-                      <button id="update_commits" class="btn btn-small">${_('Update commits')}</button>
+                      <button id="update_commits" class="btn pull-right">${_('Update commits')}</button>
+                    % else:
+                      <button class="btn disabled pull-right" disabled="disabled">${_('Update commits')}</button>
                     % endif
                     % if len(c.commit_ranges):
                       <h2>${ungettext('Compare View: %s commit','Compare View: %s commits', len(c.commit_ranges)) % len(c.commit_ranges)}</h2>
@@ -305,7 +345,7 b''
                 ${cbdiffs.render_diffset(
                   c.diffset, use_comments=True,
                   collapse_when_files_over=30,
-                  disable_new_comments=c.pull_request.is_closed())}
+                  disable_new_comments=not c.allowed_to_comment)}
 
           </div>
               % endif
@@ -313,9 +353,8 b''
 
       ## template for inline comment form
       <%namespace name="comment" file="/changeset/changeset_file_comment.html"/>
-      ${comment.comment_inline_form()}
 
-      ## render comments and inlines
+      ## render general comments
       ${comment.generate_comments(include_pull_request=True, is_pull_request=True)}
 
       % if not c.pull_request.is_closed():
@@ -394,7 +433,7 b''
                 this.closeButton.hide();
                 this.addButton.hide();
                 this.removeButtons.css('visibility', 'hidden');
-              },
+              }
             };
 
             PRDetails.init();
@@ -402,7 +441,7 b''
 
             $('#show-outdated-comments').on('click', function(e){
                 var button = $(this);
-                var outdated = $('.outdated');
+                var outdated = $('.comment-outdated');
                 if (button.html() === "(Show)") {
                   button.html("(Hide)");
                   outdated.show();
@@ -428,29 +467,6 b''
                 $(btns).each(fn_display);
             });
 
-            // inject comments into their proper positions
-            var file_comments = $('.inline-comment-placeholder');
-            %if c.pull_request.is_closed():
-                renderInlineComments(file_comments, false);
-            %else:
-                renderInlineComments(file_comments, true);
-            %endif
-            var commentTotals = {};
-            $.each(file_comments, function(i, comment) {
-                var path = $(comment).attr('path');
-                var comms = $(comment).children().length;
-                if (path in commentTotals) {
-                    commentTotals[path] += comms;
-                } else {
-                    commentTotals[path] = comms;
-                }
-            });
-            $.each(commentTotals, function(path, total) {
-                var elem = $('.comment-bubble[data-path="'+ path +'"]');
-                elem.css('visibility', 'visible');
-                elem.html(elem.html() + ' ' + total );
-            });
-
             $('#merge_pull_request_form').submit(function() {
                 if (!$('#merge_pull_request').attr('disabled')) {
                     $('#merge_pull_request').attr('disabled', 'disabled');