comments: properly show version of pull request into added comments.

- index them by numeric index instead of DB id
- show inside main comments a version in which they were made.

rhodecode/controllers/changeset.py

@@ -357,6 +357,8 b' class ChangesetController(BaseRepoContro'
                                if status else None),
                 status_change_type=status
             )
+            c.inline_comment = True if comment.line_no else False
+
             # get status if set !
             if status:
                 # if latest status was from pull request and it's closed

rhodecode/controllers/pullrequests.py

@@ -814,7 +814,11 b' class PullrequestsController(BaseRepoCon'
         _inline_cnt, c.inline_versions = cc_model.get_inline_comments_count(
             inline_comments, version=at_version, include_aggregates=True)
 
+        c.versions = pull_request_display_obj.versions()
         c.at_version_num = at_version if at_version and at_version != 'latest' else None
+        c.at_version_pos = ChangesetComment.get_index_from_version(
+            c.at_version_num, c.versions)
+
         is_outdated = lambda co: \
             not c.at_version_num \
             or co.pull_request_version_id <= c.at_version_num
@@ -872,7 +876,6 b' class PullrequestsController(BaseRepoCon'
         c.pull_request_latest = pull_request_latest
         c.at_version = at_version
 
-        c.versions = pull_request_display_obj.versions()
         c.changes = None
         c.file_changes = None
 
@@ -983,6 +986,7 b' class PullrequestsController(BaseRepoCon'
         }
         if comm:
             c.co = comm
+            c.inline_comment = True if comm.line_no else False
             data.update(comm.get_dict())
             data.update({'rendered_text':
                              render('changeset/changeset_comment_block.mako')})

rhodecode/model/db.py

@@ -2935,6 +2935,14 b' class ChangesetComment(Base, BaseModel):'
             q = q.filter(cls.pull_request_id == pull_request_id)
         return q.all()
 
+    @classmethod
+    def get_index_from_version(cls, pr_version, versions):
+        num_versions = [x.pull_request_version_id for x in versions]
+        try:
+            return num_versions.index(pr_version) +1
+        except (IndexError, ValueError):
+            return
+
     @property
     def outdated(self):
         return self.display_state == self.COMMENT_OUTDATED
@@ -2945,6 +2953,10 b' class ChangesetComment(Base, BaseModel):'
         """
         return self.outdated and self.pull_request_version_id != version
 
+    def get_index_version(self, versions):
+        return self.get_index_from_version(
+            self.pull_request_version_id, versions)
+
     def render(self, mentions=False):
         from rhodecode.lib import helpers as h
         return h.render(self.text, renderer=self.renderer, mentions=mentions)

rhodecode/public/css/comments.less

@@ -48,6 +48,12 b' tr.inline-comments div {'
 }
 
 .comment {
+
+    &.comment-general {
+        border: 1px solid @grey5;
+        padding: 5px 5px 5px 5px;
+    }
+
     margin: @padding 0;
     padding: 4px 0 0 0;
     line-height: 1em;
@@ -61,6 +67,8 b' tr.inline-comments div {'
         position: relative;
         width: 100%;
         margin: 0 0 .5em 0;
+        border-bottom: 1px solid @grey5;
+        padding: 8px 0px;
 
         &:hover .permalink {
             visibility: visible;
@@ -71,8 +79,6 b' tr.inline-comments div {'
     .author,
     .date {
         display: inline;
-        margin: 0 .5 0 0;
-        padding: 0 .5 0 0;
 
         &:after {
             content: ' | ';
@@ -80,6 +86,13 b' tr.inline-comments div {'
         }
     }
 
+    .author-general img {
+        top: -3px;
+    }
+    .author-inline img {
+        top: -3px;
+    }
+
     .status-change,
     .permalink,
     .changeset-status-lbl {
@@ -153,7 +166,6 b' tr.inline-comments div {'
 
     .text {
         clear: both;
-        border: @border-thickness solid @grey5;
         .border-radius(@border-radius);
         .box-sizing(border-box);
 
@@ -163,6 +175,19 b' tr.inline-comments div {'
           // TODO: lisa: This is needed because of other rst !important rules :[
         }
     }
+
+    .pr-version {
+        float: left;
+        margin: 0px 4px;
+    }
+    .pr-version-inline {
+        float: left;
+        margin: 1px 4px;
+    }
+    .pr-version-num {
+        font-size: 10px;
+    }
+
 }
 
 .show-outdated-comments {

rhodecode/templates/changeset/changeset_comment_block.mako

@@ -1,4 +1,4 b''
 ## this is a dummy html file for partial rendering on server and sending
 ## generated output via ajax after comment submit
 <%namespace name="comment" file="/changeset/changeset_file_comment.mako"/>
-${comment.comment_block(c.co, inline=True)}
+${comment.comment_block(c.co, inline=c.inline_comment)}

rhodecode/templates/changeset/changeset_file_comment.mako

@@ -7,9 +7,10 b''
 
 <%def name="comment_block(comment, inline=False)">
   <% outdated_at_ver = comment.outdated_at_version(getattr(c, 'at_version', None)) %>
+  <% pr_index_ver = comment.get_index_version(getattr(c, 'versions', [])) %>
 
   <div class="comment
-             ${'comment-inline' if inline else ''}
+             ${'comment-inline' if inline else 'comment-general'}
              ${'comment-outdated' if outdated_at_ver else 'comment-current'}"
        id="comment-${comment.comment_id}"
        line="${comment.line_no}"
@@ -17,27 +18,24 b''
        style="${'display: none;' if outdated_at_ver else ''}">
 
       <div class="meta">
-          <div class="author">
-              ${base.gravatar_with_user(comment.author.email, 16)}
+          <div class="author ${'author-inline' if inline else 'author-general'}">
+              ${base.gravatar_with_user(comment.author.email, 20)}
           </div>
           <div class="date">
               ${h.age_component(comment.modified_at, time_is_local=True)}
           </div>
+          % if inline:
+              <span></span>
+          % else:
           <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}:
-                          %else:
-                              ${_('Comment on pull request #%s') % comment.pull_request.pull_request_id}
-                          %endif
-                      </a>
-                  % endif
+                  <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}:
+                      % else:
+                          ${_('Comment on pull request #%s') % comment.pull_request.pull_request_id}
+                      % endif
+                  </a>
               % else:
                   % if comment.status_change:
                       ${_('Status change on commit')}:
@@ -46,15 +44,51 b''
                   % endif
               % endif
           </div>
-          %if comment.status_change:
+          % endif
+
+          % if comment.status_change:
             <div class="${'flag_status %s' % comment.status_change[0].status}"></div>
             <div title="${_('Commit status')}" class="changeset-status-lbl">
                  ${comment.status_change[0].status_lbl}
             </div>
-          %endif
+          % endif
+
           <a class="permalink" href="#comment-${comment.comment_id}"> &para;</a>
 
           <div class="comment-links-block">
+
+            % if inline:
+              % if outdated_at_ver:
+                  <div class="pr-version-inline">
+                    <a href="${h.url.current(version=comment.pull_request_version_id, anchor='comment-{}'.format(comment.comment_id))}">
+                    <code class="pr-version-num">
+                        outdated ${'v{}'.format(pr_index_ver)}
+                    </code>
+                    </a>
+                  </div>
+                  |
+              % endif
+            % else:
+                % if comment.pull_request_version_id and pr_index_ver:
+                    |
+                    <div class="pr-version">
+                      % if comment.outdated:
+                        <a href="?version=${comment.pull_request_version_id}#comment-${comment.comment_id}">
+                            ${_('Outdated comment from pull request version {}').format(pr_index_ver)}
+                        </a>
+                      % else:
+                        <div class="tooltip" title="${_('Comment from pull request version {0}').format(pr_index_ver)}">
+                            <a href="${h.url('pullrequest_show',repo_name=comment.pull_request.target_repo.repo_name,pull_request_id=comment.pull_request.pull_request_id, version=comment.pull_request_version_id)}">
+                            <code class="pr-version-num">
+                                ${'v{}'.format(pr_index_ver)}
+                            </code>
+                            </a>
+                        </div>
+                      % endif
+                    </div>
+                % endif
+            % endif
+
             ## 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, also hide delete if currently viewed comment is outdated
             %if not outdated_at_ver and (not comment.pull_request or (comment.pull_request and not comment.pull_request.is_closed())):

rhodecode/templates/codeblocks/diffs.mako

@@ -176,10 +176,10 b' collapse_all = len(diffset.files) > coll'
                     '%(num)s files changed: %(linesadd)s inserted, %(linesdel)s deleted', diffset.changed_files) % {'num': diffset.changed_files, 'linesadd': diffset.lines_added, 'linesdel': diffset.lines_deleted}}
     %endif
 
-        <% at_ver = getattr(c, 'at_version_num', None) %>
+        <% at_ver = getattr(c, 'at_version_pos', None) %>
         % if at_ver:
             <div class="pull-right">
-            ${_('Changes at version %d') % at_ver}
+            ${_('Showing changes at version %d') % at_ver}
             </div>
         % endif
 

rhodecode/templates/pullrequests/pullrequest_show.mako

@@ -203,8 +203,11 b''
 
                        ## SHOW ALL VERSIONS OF PR
                        <% ver_pr = None %>
-                       % for ver in reversed(c.pull_request.versions()):
+                       % for data in reversed(list(enumerate(c.versions, 1))):
+                       <% ver_pos = data[0] %>
+                       <% ver = data[1] %>
                        <% ver_pr = ver.pull_request_version_id %>
+
                        <tr class="version-pr" style="display: ${'' if c.at_version == ver_pr else 'none'}">
                            <td>
                                % if c.at_version == ver_pr:
@@ -214,7 +217,9 b''
                                % endif
                            </td>
                            <td>
-                               <code><a href="${h.url.current(version=ver_pr)}">version ${ver_pr}</a></code>
+                                <code class="tooltip" title="${_('Comment from pull request version {0}').format(ver_pos)}">
+                                    <a href="${h.url.current(version=ver_pr)}">v${ver_pos}</a>
+                                </code>
                            </td>
                            <td>
                                <code>${ver.source_ref_parts.commit_id[:6]}</code>