diff --git a/rhodecode/controllers/pullrequests.py b/rhodecode/controllers/pullrequests.py --- a/rhodecode/controllers/pullrequests.py +++ b/rhodecode/controllers/pullrequests.py @@ -807,49 +807,43 @@ class PullrequestsController(BaseRepoCon c.approval_msg = _('Reviewer approval is pending.') c.pr_merge_status = False - # inline comments - inline_comments = cc_model.get_inline_comments( - c.rhodecode_db_repo.repo_id, pull_request=pull_request_id) - - _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 = at_version 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 + # GENERAL COMMENTS with versions # + q = cc_model._all_general_comments_of_pull_request(pull_request_latest) + general_comments = q.order_by(ChangesetComment.pull_request_version_id.asc()) - # inline_comments_until_version - if c.at_version_num: - # if we use version, then do not show later comments - # than current version - paths = collections.defaultdict(lambda: collections.defaultdict(list)) - for fname, per_line_comments in inline_comments.iteritems(): - for lno, comments in per_line_comments.iteritems(): - for co in comments: - if co.pull_request_version_id and is_outdated(co): - paths[co.f_path][co.line_no].append(co) - inline_comments = paths + # pick comments we want to render at current version + c.comment_versions = cc_model.aggregate_comments( + general_comments, c.versions, c.at_version_num) + c.comments = c.comment_versions[c.at_version_num]['until'] + + # INLINE COMMENTS with versions # + q = cc_model._all_inline_comments_of_pull_request(pull_request_latest) + inline_comments = q.order_by(ChangesetComment.pull_request_version_id.asc()) + c.inline_versions = cc_model.aggregate_comments( + inline_comments, c.versions, c.at_version_num, inline=True) - # outdated comments - c.outdated_cnt = 0 - if CommentsModel.use_outdated_comments(pull_request_latest): - outdated_comments = cc_model.get_outdated_comments( - c.rhodecode_db_repo.repo_id, - pull_request=pull_request_at_ver) + # if we use version, then do not show later comments + # than current version + paths = 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 + # don't render comments for higher version + should_render = co.pull_request_version_id and \ + co.pull_request_version_id <= c.at_version_num + else: + # showing all, for 'latest' + should_render = True - # Count outdated comments and check for deleted files - is_outdated = lambda co: \ - not c.at_version_num \ - or co.pull_request_version_id < c.at_version_num - for file_name, lines in outdated_comments.iteritems(): - for comments in lines.values(): - comments = [comm for comm in comments if is_outdated(comm)] - c.outdated_cnt += len(comments) + if should_render: + paths[co.f_path][co.line_no].append(co) + inline_comments = paths # load compare data into template context self._load_compare_data(pull_request_at_ver, inline_comments) @@ -860,10 +854,6 @@ class PullrequestsController(BaseRepoCon # We need to swap that here to generate it properly on the html side c.target_repo = c.source_repo - # general comments - c.comments = cc_model.get_comments( - c.rhodecode_db_repo.repo_id, pull_request=pull_request_id) - if c.allowed_to_update: force_close = ('forced_closed', _('Close Pull Request')) statuses = ChangesetStatus.STATUSES + [force_close] @@ -874,7 +864,6 @@ class PullrequestsController(BaseRepoCon c.ancestor = None # TODO: add ancestor here c.pull_request = pull_request_display_obj c.pull_request_latest = pull_request_latest - c.at_version = at_version 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 @@ -39,7 +39,7 @@ from rhodecode.lib.utils import action_l from rhodecode.lib.utils2 import extract_mentioned_users from rhodecode.model import BaseModel from rhodecode.model.db import ( - ChangesetComment, User, Notification, PullRequest) + ChangesetComment, User, Notification, PullRequest, AttributeDict) from rhodecode.model.notification import NotificationModel from rhodecode.model.meta import Session from rhodecode.model.settings import VcsSettingsModel @@ -83,6 +83,52 @@ class CommentsModel(BaseModel): log.error(traceback.format_exc()) return global_renderer + def aggregate_comments(self, comments, versions, show_version, inline=False): + # group by versions, and count until, and display objects + + comment_groups = collections.defaultdict(list) + [comment_groups[ + _co.pull_request_version_id].append(_co) for _co in comments] + + def yield_comments(pos): + for co in comment_groups[pos]: + yield co + + comment_versions = collections.defaultdict( + lambda: collections.defaultdict(list)) + prev_prvid = -1 + # fake last entry with None, to aggregate on "latest" version which + # doesn't have an pull_request_version_id + for ver in versions + [AttributeDict({'pull_request_version_id': None})]: + prvid = ver.pull_request_version_id + if prev_prvid == -1: + prev_prvid = prvid + + for co in yield_comments(prvid): + comment_versions[prvid]['at'].append(co) + + # save until + current = comment_versions[prvid]['at'] + prev_until = comment_versions[prev_prvid]['until'] + cur_until = prev_until + current + comment_versions[prvid]['until'].extend(cur_until) + + # save outdated + if inline: + outdated = [x for x in cur_until + if x.outdated_at_version(show_version)] + else: + outdated = [x for x in cur_until + if x.older_than_version(show_version)] + display = [x for x in cur_until if x not in outdated] + + comment_versions[prvid]['outdated'] = outdated + comment_versions[prvid]['display'] = display + + prev_prvid = prvid + + return comment_versions + 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, @@ -376,18 +422,14 @@ class CommentsModel(BaseModel): return self._group_comments_by_path_and_line_number(q) def get_inline_comments_count(self, inline_comments, skip_outdated=True, - version=None, include_aggregates=False): - version_aggregates = collections.defaultdict(list) + version=None): inline_cnt = 0 for fname, per_line_comments in inline_comments.iteritems(): for lno, comments in per_line_comments.iteritems(): for comm in comments: - version_aggregates[comm.pull_request_version_id].append(comm) if not comm.outdated_at_version(version) and skip_outdated: inline_cnt += 1 - if include_aggregates: - return inline_cnt, version_aggregates return inline_cnt def get_outdated_comments(self, repo_id, pull_request): @@ -511,6 +553,13 @@ class CommentsModel(BaseModel): .filter(ChangesetComment.pull_request == pull_request) return comments + def _all_general_comments_of_pull_request(self, pull_request): + comments = Session().query(ChangesetComment)\ + .filter(ChangesetComment.line_no == None)\ + .filter(ChangesetComment.f_path == None)\ + .filter(ChangesetComment.pull_request == pull_request) + return comments + @staticmethod def use_outdated_comments(pull_request): settings_model = VcsSettingsModel(repo=pull_request.target_repo) diff --git a/rhodecode/model/db.py b/rhodecode/model/db.py --- a/rhodecode/model/db.py +++ b/rhodecode/model/db.py @@ -2959,6 +2959,15 @@ class ChangesetComment(Base, BaseModel): """ return self.outdated and self.pull_request_version_id != version + def older_than_version(self, version): + """ + Checks if comment is made from previous version than given + """ + if version is None: + return self.pull_request_version_id is not None + + return self.pull_request_version_id < version + @property def resolved(self): return self.resolved_by[0] if self.resolved_by else None @@ -2973,9 +2982,9 @@ class ChangesetComment(Base, BaseModel): def __repr__(self): if self.comment_id: - return '' % self.comment_id + return '' % self.comment_id else: - return '' % id(self) + return '' % id(self) class ChangesetStatus(Base, BaseModel): diff --git a/rhodecode/public/css/comments.less b/rhodecode/public/css/comments.less --- a/rhodecode/public/css/comments.less +++ b/rhodecode/public/css/comments.less @@ -51,7 +51,7 @@ tr.inline-comments div { float: left; padding: 0.4em 0.4em; - margin: 2px 5px 0px -10px; + margin: 3px 5px 0px -10px; display: inline-block; min-height: 0; diff --git a/rhodecode/templates/changeset/changeset.mako b/rhodecode/templates/changeset/changeset.mako --- a/rhodecode/templates/changeset/changeset.mako +++ b/rhodecode/templates/changeset/changeset.mako @@ -180,7 +180,7 @@ <%namespace name="comment" file="/changeset/changeset_file_comment.mako"/> ## render comments - ${comment.generate_comments()} + ${comment.generate_comments(c.comments)} ## main comment form and it status ${comment.comments(h.url('changeset_comment', repo_name=c.repo_name, revision=c.commit.raw_id), 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 @@ -6,8 +6,13 @@ <%namespace name="base" file="/base/base.mako"/> <%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', [])) %> + % if inline: + <% outdated_at_ver = comment.outdated_at_version(getattr(c, 'at_version_num', None)) %> + % else: + <% outdated_at_ver = comment.older_than_version(getattr(c, 'at_version_num', None)) %> + % endif +
% if inline: - % if outdated_at_ver:
- - outdated ${'v{}'.format(pr_index_ver)} - + % if outdated_at_ver: + + outdated ${'v{}'.format(pr_index_ver)} | + + % elif pr_index_ver: + + ${'v{}'.format(pr_index_ver)} | + + % endif
- | - % endif % else: % if comment.pull_request_version_id and pr_index_ver: | @@ -102,7 +110,7 @@ ${_('Outdated comment from pull request version {}').format(pr_index_ver)} % else: -
+
${'v{}'.format(pr_index_ver)} @@ -143,9 +151,9 @@ ## generate main comments -<%def name="generate_comments(include_pull_request=False, is_pull_request=False)"> +<%def name="generate_comments(comments, include_pull_request=False, is_pull_request=False)">
- %for comment in c.comments: + %for comment in comments:
## only render comments that are not from pull request, or from ## pull request and a status change 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 @@ -35,6 +35,7 @@ templateContext.pull_request_data.pull_request_id = ${c.pull_request.pull_request_id};
+
${self.repo_page_title(c.rhodecode_db_repo)}
@@ -42,6 +43,7 @@ ${self.breadcrumbs()}
+
<% summary = lambda n:{False:'summary-short'}.get(n) %>
@@ -173,10 +175,13 @@ ## CURRENTLY SELECT PR VERSION - % if c.at_version in [None, 'latest']: + % if c.at_version_num is None: % else: - ${len(c.inline_versions[None])} + + + ${len(c.comment_versions[None]['at'])}/${len(c.inline_versions[None]['at'])} + % endif @@ -203,36 +208,40 @@ ## SHOW ALL VERSIONS OF PR <% ver_pr = None %> + % for data in reversed(list(enumerate(c.versions, 1))): - <% ver_pos = data[0] %> - <% ver = data[1] %> - <% ver_pr = ver.pull_request_version_id %> + <% ver_pos = data[0] %> + <% ver = data[1] %> + <% ver_pr = ver.pull_request_version_id %> - - - % if c.at_version == ver_pr: - - % else: - ${len(c.inline_versions[ver_pr])} - % endif - - - - v${ver_pos} - - - - ${ver.source_ref_parts.commit_id[:6]} - - - ${_('created')} ${h.age_component(ver.updated_on)} - - - % if c.at_version == ver_pr: - ${_('Show all versions')} - % endif - - + + + % if c.at_version_num == ver_pr: + + % else: + + + ${len(c.comment_versions[ver_pr]['at'])}/${len(c.inline_versions[ver_pr]['at'])} + + % endif + + + + v${ver_pos} + + + + ${ver.source_ref_parts.commit_id[:6]} + + + ${_('created')} ${h.age_component(ver.updated_on)} + + + % if c.at_version_num == ver_pr: + ${_('Show all versions')} + % endif + + % endfor ## show comment/inline comments summary @@ -240,28 +249,39 @@ - <% inline_comm_count_ver = len(c.inline_versions[ver_pr])%> - ${_('Comments for this version')}: - %if c.comments: - ${_("%d General ") % len(c.comments)} + <% outdated_comm_count_ver = len(c.inline_versions[c.at_version_num]['outdated']) %> + <% general_outdated_comm_count_ver = len(c.comment_versions[c.at_version_num]['outdated']) %> + + + % if c.at_version: + <% inline_comm_count_ver = len(c.inline_versions[c.at_version_num]['display']) %> + <% general_comm_count_ver = len(c.comment_versions[c.at_version_num]['display']) %> + ${_('Comments at this version')}: + % else: + <% inline_comm_count_ver = len(c.inline_versions[c.at_version_num]['until']) %> + <% general_comm_count_ver = len(c.comment_versions[c.at_version_num]['until']) %> + ${_('Comments for this pull request')}: + % endif + + %if general_comm_count_ver: + ${_("%d General ") % general_comm_count_ver} %else: - ${_("%d General ") % len(c.comments)} + ${_("%d General ") % general_comm_count_ver} %endif - <% inline_comm_count_ver = len(c.inline_versions[c.at_version_num])%> %if inline_comm_count_ver: , ${_("%d Inline") % inline_comm_count_ver} %else: , ${_("%d Inline") % inline_comm_count_ver} %endif - %if c.outdated_cnt: - , ${_("%d Outdated") % c.outdated_cnt} + %if outdated_comm_count_ver: + , ${_("%d Outdated") % outdated_comm_count_ver} | ${_('show outdated comments')} %else: - , ${_("%d Outdated") % c.outdated_cnt} + , ${_("%d Outdated") % outdated_comm_count_ver} %endif @@ -459,7 +479,24 @@ Changed files: <%namespace name="comment" file="/changeset/changeset_file_comment.mako"/> ## render general comments - ${comment.generate_comments(include_pull_request=True, is_pull_request=True)} + +
+
+
+ % 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 + % endif +
+
+
+ + ${comment.generate_comments(c.comments, include_pull_request=True, is_pull_request=True)} % if not c.pull_request.is_closed(): ## main comment form and it status @@ -472,11 +509,20 @@ Changed files: