On 02/26/2016 09:54 PM, Angel Ezquerra wrote:
# HG changeset patch
# User Angel Ezquerra <[email protected]>
# Date 1456382571 -3600
# Thu Feb 25 07:42:51 2016 +0100
# Branch stable
# Node ID a13394a91f352a982afcf0b9141ece5537caacb1
# Parent 17dd4d5bff87a60b34db82e8ae9675acac138fee
summary, changelog, compare: make the comments counts more accurate
Up until now the comment counts shown on the summary, changelog and compare
pages counted both actual (non-empty) comments and status changes. This was not
particularly useful, since the status information is already shown on the
status icon. Also, openeing a changeset page to find out that it just had its
status changed (with no textual comments) is quite annoying.
Instead, show the number of "actual" comments (i.e. those that have some comment
text).
diff --git a/kallithea/controllers/changelog.py
b/kallithea/controllers/changelog.py
--- a/kallithea/controllers/changelog.py
+++ b/kallithea/controllers/changelog.py
@@ -63,6 +63,7 @@
page_revisions = [x.raw_id for x in list(c.repo_changesets)]
c.comments = c.db_repo.get_comments(page_revisions)
c.statuses = c.db_repo.statuses(page_revisions)
+ c.comment_counts = c.db_repo.count_comments(c.comments)
It seems like an odd pattern to retrieve a value from a c.db_repo method
... and then pass the same value back to another c.db_repo method for
further processing. The latter could perhaps be a class function?
(Other high level comment functions live in the comment model like
get_inline_comments is. It is already a bit inconsistent one way or the
other that get_comments lives in db. Our MVC model is quite ... not MVC.)
class ChangelogController(BaseRepoController):
@@ -153,6 +154,7 @@
page_revisions = [x.raw_id for x in c.pagination]
c.comments = c.db_repo.get_comments(page_revisions)
c.statuses = c.db_repo.statuses(page_revisions)
+ c.comment_counts = c.db_repo.count_comments(c.comments)
except EmptyRepositoryError as e:
h.flash(safe_str(e), category='warning')
return redirect(url('summary_home', repo_name=c.repo_name))
diff --git a/kallithea/controllers/compare.py b/kallithea/controllers/compare.py
--- a/kallithea/controllers/compare.py
+++ b/kallithea/controllers/compare.py
@@ -238,6 +238,7 @@
raw_ids = [x.raw_id for x in c.cs_ranges]
c.cs_comments = other_repo.get_comments(raw_ids)
c.statuses = other_repo.statuses(raw_ids)
+ c.cs_comment_counts = c.db_repo.count_comments(c.cs_comments)
revs = [ctx.revision for ctx in reversed(c.cs_ranges)]
c.jsdata = json.dumps(graph_data(c.cs_repo.scm_instance, revs))
diff --git a/kallithea/model/db.py b/kallithea/model/db.py
--- a/kallithea/model/db.py
+++ b/kallithea/model/db.py
@@ -1389,6 +1389,16 @@
grouped[cmt.revision].append(cmt)
return grouped
+ def count_comments(self, comments):
+ """
+ Returns the count of (non-empty) comments for this repository grouped
by revisions
Perhaps clarify that the purpose is to filter out comments that just are
carriers of a status change/vote?
+
+ :param comments: a {revision: comment} dict, as returned by
get_comments()
+ """
+ def count_revision_comments(revision_comments):
+ return len([cm for cm in revision_comments if cm.text])
+ return {raw_id: count_revision_comments(revision_comments) for raw_id,
revision_comments in comments.iteritems()}
This is Python 2.7 syntax - we still support 2.6. (That could perhaps
change - but that is a separate discussion.)
The local function seems a bit overkill. I would just make it:
return dict((raw_id, len([cm for cm in revision_comments if cm.text]))
for raw_id, revision_comments in comments.iteritems())
+
def statuses(self, revisions):
"""
Returns statuses for this repository.
diff --git a/kallithea/templates/changelog/changelog.html
b/kallithea/templates/changelog/changelog.html
--- a/kallithea/templates/changelog/changelog.html
+++ b/kallithea/templates/changelog/changelog.html
@@ -120,11 +120,11 @@
<div class="log-container">
<div class="message"
id="C-${cs.raw_id}">${h.urlify_commit(cs.message,
c.repo_name,h.url('changeset_home',repo_name=c.repo_name,revision=cs.raw_id))}</div>
<div class="extra-container">
- %if c.comments.get(cs.raw_id):
+ %if c.comment_counts.get(cs.raw_id, 0):
<div class="comments-container">
<div class="comments-cnt"
title="${_('Changeset has comments')}">
<a
href="${c.comments[cs.raw_id][0].url()}">
-
${len(c.comments[cs.raw_id])}
+
${c.comment_counts.get(cs.raw_id, 0)}
Here we know that the changeset is in comment_counts ... so why not just
use []? (The same applies elsewhere.)
<i
class="icon-comment-discussion"></i>
</a>
</div>
diff --git a/kallithea/templates/changelog/changelog_summary_data.html
b/kallithea/templates/changelog/changelog_summary_data.html
--- a/kallithea/templates/changelog/changelog_summary_data.html
+++ b/kallithea/templates/changelog/changelog_summary_data.html
@@ -31,11 +31,11 @@
</div>
</td>
<td class="compact">
- %if c.comments.get(cs.raw_id,[]):
+ %if c.comment_counts.get(cs.raw_id, 0):
<div class="comments-container">
<div title="${('comments')}">
- <a href="${c.comments[cs.raw_id][0].url()}">
- <i
class="icon-comment"></i>${len(c.comments[cs.raw_id])}
+ <a href="${c.comments[cs.raw_id][0].url()}">
+ <i
class="icon-comment"></i>${c.comment_counts.get(cs.raw_id, 0)}
</a>
</div>
</div>
diff --git a/kallithea/templates/compare/compare_cs.html
b/kallithea/templates/compare/compare_cs.html
--- a/kallithea/templates/compare/compare_cs.html
+++ b/kallithea/templates/compare/compare_cs.html
@@ -25,11 +25,11 @@
<i class="icon-circle
changeset-status-${c.statuses[cs.raw_id][0]}"></i>
</div>
%endif
- %if c.cs_comments.get(cs.raw_id):
+ %if c.cs_comment_counts.get(cs.raw_id, 0):
<div class="comments-container">
<div class="comments-cnt" title="${_('Changeset has
comments')}">
<a href="${c.cs_comments[cs.raw_id][0].url()}">
- ${len(c.cs_comments[cs.raw_id])}
+ ${c.cs_comment_counts.get(cs.raw_id, 0)}
<i class="icon-comment"></i>
</a>
</div>
_______________________________________________
kallithea-general mailing list
[email protected]
http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
_______________________________________________
kallithea-general mailing list
[email protected]
http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
