<html>
<head>
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<div class="moz-cite-prefix">On 02/26/2016 09:54 PM, Angel Ezquerra
wrote:<br>
</div>
<blockquote
cite="mid:a13394a91f352a982afc.1456520085@ezquerra.agp.is.keysight.com"
type="cite">
<pre wrap=""># HG changeset patch
# User Angel Ezquerra <a class="moz-txt-link-rfc2396E" href="mailto:angel.ezquerra@gmail.com"><angel.ezquerra@gmail.com></a>
# 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)</pre>
</blockquote>
<br>
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?<br>
<br>
(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.)<br>
<br>
<blockquote
cite="mid:a13394a91f352a982afc.1456520085@ezquerra.agp.is.keysight.com"
type="cite">
<pre wrap="">
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</pre>
</blockquote>
<br>
Perhaps clarify that the purpose is to filter out comments that just
are carriers of a status change/vote?<br>
<br>
<blockquote
cite="mid:a13394a91f352a982afc.1456520085@ezquerra.agp.is.keysight.com"
type="cite">
<pre wrap="">
+
+ :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()}</pre>
</blockquote>
<br>
This is Python 2.7 syntax - we still support 2.6. (That could
perhaps change - but that is a separate discussion.)<br>
<br>
The local function seems a bit overkill. I would just make it:<br>
<br>
<pre wrap=""> return dict((raw_id, len([cm for cm in revision_comments if cm.text]))
for raw_id, revision_comments in comments.iteritems())</pre>
<br>
<blockquote
cite="mid:a13394a91f352a982afc.1456520085@ezquerra.agp.is.keysight.com"
type="cite">
<pre wrap="">+
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)}</pre>
</blockquote>
<br>
Here we know that the changeset is in comment_counts ... so why not
just use []? (The same applies elsewhere.)<br>
<br>
<blockquote
cite="mid:a13394a91f352a982afc.1456520085@ezquerra.agp.is.keysight.com"
type="cite">
<pre wrap=""> <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>
</pre>
<br>
<fieldset class="mimeAttachmentHeader"></fieldset>
<br>
<pre wrap="">_______________________________________________
kallithea-general mailing list
<a class="moz-txt-link-abbreviated" href="mailto:kallithea-general@sfconservancy.org">kallithea-general@sfconservancy.org</a>
<a class="moz-txt-link-freetext" href="http://lists.sfconservancy.org/mailman/listinfo/kallithea-general">http://lists.sfconservancy.org/mailman/listinfo/kallithea-general</a>
</pre>
</blockquote>
<br>
</body>
</html>