Merge lp:~abentley/launchpad/bugcomment-as-icomment into lp:launchpad

[Aaron Bentley]

Use common rendering for BugComment and CodeReviewComment body text.

= Summary =
Truncate long code review comments and provide "Read more..." link.

== Proposed fix ==
Use common rendering for BugComment and CodeReviewComment

== Pre-implementation notes ==
Discussed with deryck

== Implementation details ==
Provide MessageComment to implement IComment in terms of IMessage, and use in CodeReivewComment, BugComment, DistroSeriesDifferenceDisplayComment. Provide adapters to convert CodeReviewComment and DistroSeriesDifferenceDisplayComment into IMessage.

Move the code for truncation/Read More from bug comments to lp.services.comments.

IComment gains too_long and text_for_display attributes. MessageComment implements them.

Support truncation to 3200 chars in CodeReviewDisplayComment.

Adjust CodeReviewDisplayComment call sites to specify whether to truncate.

Use a single "comment-text" css class, instead of multiple ones.

Add browser_open to harness.py, to simplify interactive testing.

== Tests ==
bin/test -t test_bugcomment -t bugcomment.txt -t xx-bug-comments-truncated.txt -t xx-bug-index-lots-of-comments.txt -t xx-numbered-comments.txt -t test_branchmergeproposal -t test_codereviewcomment -t conversation.txt

== Demo and Q/A ==
Create a code review comment longer than 3200 chars. It should be truncated and have a "Read more" link. That link should display the full code review comment.

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/services/comments/browser/configure.zcml
  lib/lp/registry/browser/configure.zcml
  lib/lp/bugs/browser/tests/test_bugcomment.py
  lib/lp/code/browser/configure.zcml
  lib/lp/bugs/stories/bugs/xx-bug-comments-truncated.txt
  lib/lp/code/browser/branch.py
  lib/lp/code/templates/codereviewcomment-body.pt
  lib/canonical/launchpad/icing/css/modifiers.css
  lib/lp/code/browser/tests/test_branchmergeproposal.py
  lib/lp/code/browser/tests/test_codereviewcomment.py
  lib/lp/bugs/browser/bugcomment.py
  lib/lp/services/comments/browser/messagecomment.py
  lib/lp/registry/browser/distroseriesdifference.py
  lib/lp/testing/factory.py
  lib/lp/services/comments/interfaces/conversation.py
  lib/lp/code/browser/codereviewcomment.py
  lib/lp/bugs/interfaces/bugmessage.py
  lib/lp/scripts/harness.py
  lib/lp/bugs/stories/bugs/xx-numbered-comments.txt
  lib/lp/services/comments/templates/comment-body.pt
  lib/lp/bugs/doc/bugcomment.txt
  lib/lp/services/comments/doc/conversation.txt
  lib/lp/code/browser/branchmergeproposal.py
  lib/lp/bugs/templates/bugcomment-box.pt
  lib/lp/bugs/stories/bugs/xx-bug-index-lots-of-comments.txt

./lib/canonical/launchpad/icing/css/modifiers.css
      61: Unknown Property name.: filter
      62: Unknown Property name.: -ms-filter
     140: Unknown Property name.: filter
     141: Unknown Property name.: -ms-filter
      61: I009: Wrong separator on property: value pair.
     140: I009: Wrong separator on property: value pair.
./lib/lp/scripts/harness.py
      26: 'from storm.expr import *' used; unable to detect undefined names
      28: 'from storm.locals import *' used; unable to detect undefined names
      21: 'rlcompleter' imported but unused

[Brad Crittenden (bac)]

Aaron this is a really nice branch. It was very readable and understandable. Thanks for the nice work. I've got no suggested changes or questions.

review: Approve (code)

=== modified file 'lib/canonical/launchpad/icing/css/modifiers.css'
--- lib/canonical/launchpad/icing/css/modifiers.css	2012-01-26 14:20:33 +0000
+++ lib/canonical/launchpad/icing/css/modifiers.css	2012-01-26 14:20:34 +0000
@@ -155,9 +155,8 @@
 
 pre.changelog,
 table.diff,
-.bug-comment,
-.bug-activity,
-.codereviewcomment {
+.comment-text,
+.bug-activity {
     font-family: 'Ubuntu Mono', monospace;
     }
 

=== modified file 'lib/lp/bugs/browser/bugcomment.py'
--- lib/lp/bugs/browser/bugcomment.py	2012-01-06 19:45:52 +0000
+++ lib/lp/bugs/browser/bugcomment.py	2012-01-26 14:20:34 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# Copyright 2006-2012 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Bug comment browser view classes."""
@@ -41,6 +41,7 @@
 
 from lp.bugs.interfaces.bugattachment import BugAttachmentType
 from lp.bugs.interfaces.bugmessage import IBugComment
+from lp.services.comments.browser.messagecomment import MessageComment
 from lp.services.config import config
 from lp.services.features import getFeatureFlag
 from lp.services.librarian.browser import ProxiedLibraryFileAlias
@@ -176,7 +177,7 @@
             yield [event for (kind, event) in window_group]
 
 
-class BugComment:
+class BugComment(MessageComment):
     """Data structure that holds all data pertaining to a bug comment.
 
     It keeps track of which index it has in the bug comment list and
@@ -194,6 +195,11 @@
     def __init__(
             self, index, message, bugtask, activity=None,
             show_spam_controls=False, user=None, display='full'):
+        if display == 'truncate':
+            comment_limit = config.malone.max_comment_size
+        else:
+            comment_limit = None
+        super(BugComment, self).__init__(comment_limit)
 
         self.index = index
         self.bugtask = bugtask
@@ -216,10 +222,6 @@
         if bool(getFeatureFlag(flag)):
             user_owns_comment = user is not None and user == self.owner
         self.show_spam_controls = show_spam_controls or user_owns_comment
-        if display == 'truncate':
-            self.comment_limit = config.malone.max_comment_size
-        else:
-            self.comment_limit = None
         self.hide_text = (display == 'hide')
 
     @cachedproperty
@@ -239,28 +241,12 @@
         """
         return not self.visible
 
-    @property
-    def needs_truncation(self):
-        if self.comment_limit is None:
-            return False
-        return len(self.text_contents) > self.comment_limit
-
-    @property
-    def was_truncated(self):
-        return self.needs_truncation
-
     @cachedproperty
     def text_for_display(self):
         if self.hide_text:
             return ''
-        if not self.needs_truncation:
-            return self.text_contents
-        # Note here that we truncate at comment_limit, and not
-        # comment_limit - 3; while it would be nice to account for
-        # the ellipsis, this breaks down when the comment limit is
-        # less than 3 (which can happen in a testcase) and it makes
-        # counting the strings harder.
-        return "%s..." % self.text_contents[:self.comment_limit]
+        else:
+            return super(BugComment, self).text_for_display
 
     def isIdenticalTo(self, other):
         """Compare this BugComment to another and return True if they are

=== modified file 'lib/lp/bugs/browser/tests/test_bugcomment.py'
--- lib/lp/bugs/browser/tests/test_bugcomment.py	2012-01-06 19:45:52 +0000
+++ lib/lp/bugs/browser/tests/test_bugcomment.py	2012-01-26 14:20:34 +0000
@@ -1,4 +1,4 @@
-# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# Copyright 2010-2012 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Tests for the bugcomment module."""

=== modified file 'lib/lp/bugs/doc/bugcomment.txt'
--- lib/lp/bugs/doc/bugcomment.txt	2012-01-26 14:20:33 +0000
+++ lib/lp/bugs/doc/bugcomment.txt	2012-01-26 14:20:34 +0000
@@ -115,7 +115,7 @@
 If we get the bug comments from the view we can see that the two additional
 comments have been truncated:
 
-    >>> [(bug_comment.index, bug_comment.was_truncated)
+    >>> [(bug_comment.index, bug_comment.too_long)
     ...  for bug_comment in bug_comments(bug_view)]
     [(1, True), (2, True)]
 

=== modified file 'lib/lp/bugs/interfaces/bugmessage.py'
--- lib/lp/bugs/interfaces/bugmessage.py	2012-01-01 02:58:52 +0000
+++ lib/lp/bugs/interfaces/bugmessage.py	2012-01-26 14:20:34 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# Copyright 2004-2012 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 # pylint: disable-msg=E0211,E0213
@@ -32,6 +32,7 @@
 from lp.bugs.interfaces.hasbug import IHasBug
 from lp.registry.interfaces.person import IPerson
 from lp.services.fields import Title
+from lp.services.comments.interfaces.conversation import IComment
 from lp.services.messages.interfaces.message import IMessage
 
 
@@ -113,7 +114,7 @@
         required=False, default=None)
 
 
-class IBugComment(IMessage):
+class IBugComment(IMessage, IComment):
     """A bug comment for displaying in the web UI."""
 
     bugtask = Attribute(
@@ -127,11 +128,6 @@
         title=u'A hidden comment still displayed for admins.',
         readonly=True)
     index = Int(title=u'The comment number', required=True, readonly=True)
-    was_truncated = Bool(
-        title=u'Whether the displayed text was truncated for display.',
-        readonly=True)
-    text_for_display = Text(
-        title=u'The comment text to be displayed in the UI.', readonly=True)
     display_title = Attribute('Whether or not to show the title.')
     synchronized = Attribute(
         'Has the comment been synchronized with a remote bug tracker?')

=== modified file 'lib/lp/bugs/stories/bugs/xx-bug-comments-truncated.txt'
--- lib/lp/bugs/stories/bugs/xx-bug-comments-truncated.txt	2012-01-26 14:20:33 +0000
+++ lib/lp/bugs/stories/bugs/xx-bug-comments-truncated.txt	2012-01-26 14:20:34 +0000
@@ -32,14 +32,14 @@
     ...         print div_tag
     >>> print_comments(browser.contents) #doctest: -ELLIPSIS
     <div class="boardCommentBody">
-    <div class="bug-comment" itemprop="commentText"><p>This
+    <div class="comment-text" itemprop="commentText"><p>This
     would be a real killer feature. If there...</p></div>
     <p>
     <a href="/tomcat/+bug/2/comments/1">Read more...</a>
     </p>
     </div>
     <div class="boardCommentBody">
-    <div class="bug-comment" itemprop="commentText"><p>Oddly
+    <div class="comment-text" itemprop="commentText"><p>Oddly
     enough the bug system seems only capabl...</p></div>
     <p>
     <a href="/tomcat/+bug/2/comments/2">Read more...</a>
@@ -59,7 +59,7 @@
 
     >>> print_comments(browser.contents) #doctest: -ELLIPSIS
     <div class="boardCommentBody">
-    <div class="bug-comment" itemprop="commentText"><p>This
+    <div class="comment-text" itemprop="commentText"><p>This
       would be a real killer feature. If there is already code
       to make it possible, why aren't there tons of press announcements
       about the secuirty possibilities. Imagine - no more embarrassing

=== modified file 'lib/lp/bugs/stories/bugs/xx-bug-index-lots-of-comments.txt'
--- lib/lp/bugs/stories/bugs/xx-bug-index-lots-of-comments.txt	2011-12-29 05:29:36 +0000
+++ lib/lp/bugs/stories/bugs/xx-bug-index-lots-of-comments.txt	2012-01-26 14:20:34 +0000
@@ -8,7 +8,7 @@
 Bug 11 has quite a few comments.
 
     >>> user_browser.open('http://launchpad.dev/bugs/11')
-    >>> comments = find_tags_by_class(user_browser.contents, 'bug-comment')
+    >>> comments = find_tags_by_class(user_browser.contents, 'comment-text')
     >>> len(comments)
     6
 
@@ -26,7 +26,7 @@
 Now only 3 comments will be displayed; the oldest and the 2 newest.
 
     >>> user_browser.open('http://launchpad.dev/bugs/11')
-    >>> comments = find_tags_by_class(user_browser.contents, 'bug-comment')
+    >>> comments = find_tags_by_class(user_browser.contents, 'comment-text')
     >>> len(comments)
     3
 
@@ -59,7 +59,7 @@
     'http://.../jokosher/+bug/11?comments=all'
 
     >>> user_browser.getLink('View all 6 comments').click()
-    >>> comments = find_tags_by_class(user_browser.contents, 'bug-comment')
+    >>> comments = find_tags_by_class(user_browser.contents, 'comment-text')
     >>> len(comments)
     6
 

=== modified file 'lib/lp/bugs/stories/bugs/xx-numbered-comments.txt'
--- lib/lp/bugs/stories/bugs/xx-numbered-comments.txt	2011-10-20 00:53:01 +0000
+++ lib/lp/bugs/stories/bugs/xx-numbered-comments.txt	2012-01-26 14:20:34 +0000
@@ -13,7 +13,7 @@
     ...     number_node = comment.find(None, 'bug-comment-index')
     ...     person_node = comment.find(
     ...         lambda node: 'person' in node.get('class', ''))
-    ...     comment_node = comment.find(None, 'bug-comment')
+    ...     comment_node = comment.find(None, 'comment-text')
     ...     print "%s: %s\n  %s" % (
     ...         extract_text(number_node),
     ...         extract_text(person_node),

=== modified file 'lib/lp/bugs/templates/bugcomment-box.pt'
--- lib/lp/bugs/templates/bugcomment-box.pt	2012-01-11 10:07:56 +0000
+++ lib/lp/bugs/templates/bugcomment-box.pt	2012-01-26 14:20:34 +0000
@@ -90,14 +90,8 @@
       </li>
     </ul>
 
-    <div class="bug-comment" itemprop="commentText"
-      tal:content="structure
-        comment/text_for_display/fmt:obfuscate-email/fmt:email-to-html">
-      Comment text.
-    </div>
-    <p tal:condition="comment/was_truncated">
-      <a tal:attributes="href comment/fmt:url">Read more...</a>
-    </p>
+    <tal:text
+      replace="structure comment/@@+comment-body-text" />
   </div>
 
   <div class="boardCommentFooter" tal:condition="comment/show_footer">

=== modified file 'lib/lp/code/browser/branch.py'
--- lib/lp/code/browser/branch.py	2011-12-30 06:14:56 +0000
+++ lib/lp/code/browser/branch.py	2012-01-26 14:20:34 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2011 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Branch views."""
@@ -1314,7 +1314,7 @@
     for_input = True
 
     custom_widget('target_branch', TargetBranchWidget)
-    custom_widget('comment', TextAreaWidget, cssClass='codereviewcomment')
+    custom_widget('comment', TextAreaWidget, cssClass='comment-text')
 
     page_title = label = 'Propose branch for merging'
 

=== modified file 'lib/lp/code/browser/branchmergeproposal.py'
--- lib/lp/code/browser/branchmergeproposal.py	2012-01-01 02:58:52 +0000
+++ lib/lp/code/browser/branchmergeproposal.py	2012-01-26 14:20:34 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2011 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 # pylint: disable-msg=C0322,F0401
@@ -573,6 +573,8 @@
         self.extra_css_class = None
         self.comment_author = None
         self.body_text = None
+        self.text_for_display = None
+        self.too_long = False
         self.comment_date = None
         self.display_attachments = False
 
@@ -648,7 +650,8 @@
         while merge_proposal is not None:
             from_superseded = merge_proposal != self.context
             comments.extend(
-                CodeReviewDisplayComment(comment, from_superseded)
+                CodeReviewDisplayComment(
+                    comment, from_superseded, limit_length=True)
                 for comment in merge_proposal.all_comments)
             merge_proposal = merge_proposal.supersedes
         comments = sorted(comments, key=operator.attrgetter('date'))
@@ -1426,7 +1429,7 @@
     schema = IAddVote
     field_names = ['vote', 'review_type', 'comment']
 
-    custom_widget('comment', TextAreaWidget, cssClass='codereviewcomment')
+    custom_widget('comment', TextAreaWidget, cssClass='comment-text')
 
     @cachedproperty
     def initial_values(self):

=== modified file 'lib/lp/code/browser/codereviewcomment.py'
--- lib/lp/code/browser/codereviewcomment.py	2012-01-01 02:58:52 +0000
+++ lib/lp/code/browser/codereviewcomment.py	2012-01-26 14:20:34 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2011 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __metaclass__ = type
@@ -33,6 +33,7 @@
 from lp.code.interfaces.codereviewcomment import ICodeReviewComment
 from lp.code.interfaces.codereviewvote import ICodeReviewVoteReference
 from lp.services.comments.interfaces.conversation import IComment
+from lp.services.comments.browser.messagecomment import MessageComment
 from lp.services.config import config
 from lp.services.librarian.interfaces import ILibraryFileAlias
 from lp.services.propertycache import (
@@ -52,7 +53,7 @@
     """Marker interface for displaying code review comments."""
 
 
-class CodeReviewDisplayComment:
+class CodeReviewDisplayComment(MessageComment):
     """A code review comment or activity or both.
 
     The CodeReviewComment itself does not implement the IComment interface as
@@ -64,7 +65,12 @@
 
     delegates(ICodeReviewComment, 'comment')
 
-    def __init__(self, comment, from_superseded=False):
+    def __init__(self, comment, from_superseded=False, limit_length=True):
+        if limit_length:
+            comment_limit = config.malone.max_comment_size
+        else:
+            comment_limit = None
+        super(CodeReviewDisplayComment, self).__init__(comment_limit)
         self.comment = comment
         get_property_cache(self).has_body = bool(self.comment.message_body)
         self.has_footer = self.comment.vote is not None
@@ -80,26 +86,11 @@
             return ''
 
     @cachedproperty
-    def comment_author(self):
-        """The author of the comment."""
-        return self.comment.message.owner
-
-    @cachedproperty
-    def has_body(self):
-        """Is there body text?"""
-        return bool(self.body_text)
-
-    @cachedproperty
     def body_text(self):
         """Get the body text for the message."""
         return self.comment.message_body
 
     @cachedproperty
-    def comment_date(self):
-        """The date of the comment."""
-        return self.comment.message.datecreated
-
-    @cachedproperty
     def all_attachments(self):
         return self.comment.getAttachments()
 
@@ -114,6 +105,11 @@
         return self.all_attachments[1]
 
 
+def get_message(display_comment):
+    """Adapt an ICodeReviwComment to an IMessage."""
+    return display_comment.comment.message
+
+
 class CodeReviewCommentPrimaryContext:
     """The primary context is the comment is that of the source branch."""
 
@@ -173,7 +169,7 @@
     @cachedproperty
     def comment(self):
         """The decorated code review comment."""
-        return CodeReviewDisplayComment(self.context)
+        return CodeReviewDisplayComment(self.context, limit_length=False)
 
     @property
     def page_description(self):
@@ -239,7 +235,7 @@
 
     schema = IEditCodeReviewComment
 
-    custom_widget('comment', TextAreaWidget, cssClass='codereviewcomment')
+    custom_widget('comment', TextAreaWidget, cssClass='comment-text')
     custom_widget('vote', MyDropWidget)
 
     page_title = 'Reply to code review comment'

=== modified file 'lib/lp/code/browser/configure.zcml'
--- lib/lp/code/browser/configure.zcml	2012-01-09 11:36:12 +0000
+++ lib/lp/code/browser/configure.zcml	2012-01-26 14:20:34 +0000
@@ -1,4 +1,4 @@
-<!-- Copyright 2009-2011 Canonical Ltd.  This software is licensed under the
+<!-- Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
      GNU Affero General Public License version 3 (see the file LICENSE).
 -->
 
@@ -666,6 +666,11 @@
             name="+comment-footer"
             template="../templates/codereviewcomment-footer.pt"/>
     </browser:pages>
+    <adapter
+      provides="lp.services.messages.interfaces.message.IMessage"
+      for="lp.code.browser.codereviewcomment.ICodeReviewDisplayComment"
+      factory="lp.code.browser.codereviewcomment.get_message"/>
+
     <browser:pages
         for="lp.code.browser.branchmergeproposal.ICodeReviewNewRevisions"
         layer="lp.code.publisher.CodeLayer"

=== modified file 'lib/lp/code/browser/tests/test_branchmergeproposal.py'
--- lib/lp/code/browser/tests/test_branchmergeproposal.py	2012-01-01 02:58:52 +0000
+++ lib/lp/code/browser/tests/test_branchmergeproposal.py	2012-01-26 14:20:34 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2011 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 # pylint: disable-msg=F0401
@@ -1114,6 +1114,22 @@
                 "automatically when ready.",
             browser.contents)
 
+    def test_short_conversation_comments_not_truncated(self):
+        """Short comments should not be truncated."""
+        comment = self.factory.makeCodeReviewComment(body='x y' * 100)
+        browser = self.getViewBrowser(comment.branch_merge_proposal)
+        self.assertIn('x y' * 100, browser.contents)
+
+    def test_long_conversation_comments_truncated(self):
+        """Long comments in a conversation should be truncated."""
+        comment = self.factory.makeCodeReviewComment(body='x y' * 2000)
+        url = canonical_url(comment, force_local_path=True)
+        browser = self.getViewBrowser(comment.branch_merge_proposal)
+        self.assertNotIn('x y' * 2000, browser.contents)
+        read_more = Tag(
+            'Read more link', 'a', {'href': url}, text='Read more...')
+        self.assertThat(browser.contents, HTMLContains(read_more))
+
 
 class TestLatestProposalsForEachBranch(TestCaseWithFactory):
     """Confirm that the latest branch is returned."""

=== modified file 'lib/lp/code/browser/tests/test_codereviewcomment.py'
--- lib/lp/code/browser/tests/test_codereviewcomment.py	2012-01-01 02:58:52 +0000
+++ lib/lp/code/browser/tests/test_codereviewcomment.py	2012-01-26 14:20:34 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Unit tests for CodeReviewComments."""
@@ -70,3 +70,10 @@
                 dict(
                     name='description',
                     content=comment.message_body))))
+
+    def test_long_comments_not_truncated(self):
+        """Long comments displayed by themselves are not truncated."""
+        comment = self.factory.makeCodeReviewComment(body='x y' * 2000)
+        browser = self.getViewBrowser(comment)
+        body = Tag('Body text', 'p', text='x y' * 2000)
+        self.assertThat(browser.contents, HTMLContains(body))

=== modified file 'lib/lp/code/templates/codereviewcomment-body.pt'
--- lib/lp/code/templates/codereviewcomment-body.pt	2011-12-22 09:05:46 +0000
+++ lib/lp/code/templates/codereviewcomment-body.pt	2012-01-26 14:20:34 +0000
@@ -3,8 +3,7 @@
    xmlns:metal="http://xml.zope.org/namespaces/metal"
    omit-tag="">
 
-  <tal:message replace="structure view/comment/body_text/fmt:obfuscate-email/fmt:nice_pre" />
-
+  <div tal:content="structure context/@@+comment-body-text" />
   <tal:good-attachments repeat="attachment view/comment/display_attachments">
     <div class="boardComment attachment">
       <div class="boardCommentDetails filename"><a tal:content="attachment/filename" tal:attributes="href attachment/getURL"/></div>

=== modified file 'lib/lp/registry/browser/configure.zcml'
--- lib/lp/registry/browser/configure.zcml	2012-01-18 16:49:09 +0000
+++ lib/lp/registry/browser/configure.zcml	2012-01-26 14:20:34 +0000
@@ -1,4 +1,4 @@
-<!-- Copyright 2009-2011 Canonical Ltd.  This software is licensed under the
+<!-- Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
      GNU Affero General Public License version 3 (see the file LICENSE).
 -->
 
@@ -2238,4 +2238,8 @@
         attribute_to_parent="owner" />
 
 </facet>
+<adapter
+  provides="lp.services.messages.interfaces.message.IMessage"
+  for="lp.registry.browser.distroseriesdifference.IDistroSeriesDifferenceDisplayComment"
+  factory="lp.registry.browser.distroseriesdifference.get_message"/>
 </configure>

=== modified file 'lib/lp/registry/browser/distroseriesdifference.py'
--- lib/lp/registry/browser/distroseriesdifference.py	2012-01-17 21:45:24 +0000
+++ lib/lp/registry/browser/distroseriesdifference.py	2012-01-26 14:20:34 +0000
@@ -45,6 +45,7 @@
     IComment,
     IConversation,
     )
+from lp.services.comments.browser.messagecomment import MessageComment
 from lp.services.propertycache import cachedproperty
 from lp.services.webapp import (
     LaunchpadView,
@@ -241,20 +242,23 @@
             self.show_package_diffs_request_link)
 
 
-class DistroSeriesDifferenceDisplayComment:
+class IDistroSeriesDifferenceDisplayComment(IComment):
+    """Marker interface."""
+
+
+class DistroSeriesDifferenceDisplayComment(MessageComment):
     """Used simply to provide `IComment` for rendering."""
-    implements(IComment)
-
-    has_body = True
-    has_footer = False
-    display_attachments = False
-    extra_css_class = ''
+    implements(IDistroSeriesDifferenceDisplayComment)
 
     def __init__(self, comment):
         """Setup the attributes required by `IComment`."""
-        self.comment_author = comment.comment_author
-        self.comment_date = comment.comment_date
-        self.body_text = comment.body_text
+        super(DistroSeriesDifferenceDisplayComment, self).__init__(None)
+        self.comment = comment
+
+
+def get_message(comment):
+    """Adapter from IDistroSeriesDifferenceDisplayComment to IMessage."""
+    return comment.comment.message
 
 
 class CommentXHTMLRepresentation(LaunchpadView):

=== modified file 'lib/lp/scripts/harness.py'
--- lib/lp/scripts/harness.py	2012-01-26 14:20:33 +0000
+++ lib/lp/scripts/harness.py	2012-01-26 14:20:34 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2011 Canonical Ltd.  This software is licensed under the
+# Copyright 2004-2012 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Scripts for starting a Python prompt with Launchpad initialized.
@@ -20,6 +20,7 @@
 import readline
 import rlcompleter
 import sys
+import webbrowser
 
 from pytz import utc
 from storm.expr import *
@@ -87,6 +88,17 @@
     # Having a factory instance is handy.
     factory = LaunchpadObjectFactory()
 
+    def browser_open(obj, *args, **kwargs):
+        """Open a (possibly newly-created) object's view in a web browser.
+
+        Accepts the same parameters as canonical_url.
+
+        Performs a commit before invoking the browser, so
+        "browser_open(factory.makeFoo())" works.
+        """
+        transaction.commit()
+        webbrowser.open(canonical_url(obj, *args, **kwargs))
+
     # Silence unused name warnings
     factory, store
 

=== modified file 'lib/lp/services/comments/browser/configure.zcml'
--- lib/lp/services/comments/browser/configure.zcml	2010-09-17 11:42:29 +0000
+++ lib/lp/services/comments/browser/configure.zcml	2012-01-26 14:20:34 +0000
@@ -1,5 +1,6 @@
-<!-- Copyright 2009 Canonical Ltd.  This software is licensed under the
-     GNU Affero General Public License version 3 (see the file LICENSE).
+<!-- Copyright 2009, 2010, 2012 Canonical Ltd.  This software is licensed
+     under the GNU Affero General Public License version 3 (see the file
+     LICENSE).
 -->
 
 <configure
@@ -24,9 +25,14 @@
       <browser:page
           name="+comment-header"
           template="../templates/comment-header.pt"/>
+      <!-- Default comment body is just the body text, but implementations may
+      vary -->
       <browser:page
           name="+comment-body"
           template="../templates/comment-body.pt"/>
+      <browser:page
+          name="+comment-body-text"
+          template="../templates/comment-body.pt"/>
   </browser:pages>
 
 </configure>

=== added file 'lib/lp/services/comments/browser/messagecomment.py'
--- lib/lp/services/comments/browser/messagecomment.py	1970-01-01 00:00:00 +0000
+++ lib/lp/services/comments/browser/messagecomment.py	2012-01-26 14:20:34 +0000
@@ -0,0 +1,61 @@
+# Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+__metaclass__ = type
+
+__all__ = ['MessageComment']
+
+
+from lp.services.messages.interfaces.message import IMessage
+from lp.services.propertycache import cachedproperty
+
+
+class MessageComment:
+    """Mixin to partially implement IComment in terms of IMessage."""
+
+    extra_css_class = ''
+
+    has_footer = False
+
+    def __init__(self, comment_limit):
+        self.comment_limit = comment_limit
+
+    @property
+    def display_attachments(self):
+        return []
+
+    @cachedproperty
+    def comment_author(self):
+        """The author of the comment."""
+        return IMessage(self).owner
+
+    @cachedproperty
+    def has_body(self):
+        """Is there body text?"""
+        return bool(self.body_text)
+
+    @cachedproperty
+    def comment_date(self):
+        """The date of the comment."""
+        return IMessage(self).datecreated
+
+    @property
+    def body_text(self):
+        return IMessage(self).text_contents
+
+    @property
+    def too_long(self):
+        if self.comment_limit is None:
+            return False
+        return len(self.body_text) > self.comment_limit
+
+    @cachedproperty
+    def text_for_display(self):
+        if not self.too_long:
+            return self.body_text
+        # Note here that we truncate at comment_limit, and not
+        # comment_limit - 3; while it would be nice to account for
+        # the ellipsis, this breaks down when the comment limit is
+        # less than 3 (which can happen in a testcase) and it makes
+        # counting the strings harder.
+        return "%s..." % self.body_text[:self.comment_limit]

=== modified file 'lib/lp/services/comments/doc/conversation.txt'
--- lib/lp/services/comments/doc/conversation.txt	2012-01-26 14:20:33 +0000
+++ lib/lp/services/comments/doc/conversation.txt	2012-01-26 14:20:34 +0000
@@ -36,7 +36,10 @@
  * +comment-header - the top part of the comment
      normally something like "Bob wrote 4 seconds ago"
  * +comment-body - the main content
-     Only rendered if IComment.has_body is true.
+     Only rendered if IComment.has_body is true.  By default, is the same as
+     +comment-body-text
+ * +comment-body-text - the main content
+     The textual portion of the message body, suitably escaped and linkified.
  * +comment-footer - associated activity or other footer info, like
      bug activity or code reviews.
      Only rendered if IComment.has_footer is true

=== modified file 'lib/lp/services/comments/interfaces/conversation.py'
--- lib/lp/services/comments/interfaces/conversation.py	2011-12-24 16:54:44 +0000
+++ lib/lp/services/comments/interfaces/conversation.py	2012-01-26 14:20:34 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Interfaces to do with conversations on Launchpad entities."""
@@ -39,6 +39,13 @@
         description=_("Does the comment have a footer?"),
         readonly=True)
 
+    too_long = Bool(
+        title=u'Whether the comment body is too long to display in full.',
+        readonly=True)
+
+    text_for_display = Text(
+        title=u'The comment text to be displayed in the UI.', readonly=True)
+
     body_text = Text(
         description=_("The body text of the comment."),
         readonly=True)

=== modified file 'lib/lp/services/comments/templates/comment-body.pt'
--- lib/lp/services/comments/templates/comment-body.pt	2010-09-17 10:50:51 +0000
+++ lib/lp/services/comments/templates/comment-body.pt	2012-01-26 14:20:34 +0000
@@ -3,6 +3,10 @@
    xmlns:metal="http://xml.zope.org/namespaces/metal"
    omit-tag="">
 
-  <tal:message replace="structure context/body_text/fmt:obfuscate-email/fmt:nice_pre" />
+  <div class="comment-text" itemprop="commentText" tal:content="structure
+  context/text_for_display/fmt:obfuscate-email/fmt:email-to-html" />
+  <p tal:condition="context/too_long">
+    <a tal:attributes="href context/fmt:url">Read more...</a>
+  </p>
 
 </tal:root>

=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py	2012-01-19 14:36:19 +0000
+++ lib/lp/testing/factory.py	2012-01-26 14:20:34 +0000
@@ -2,7 +2,7 @@
 # NOTE: The first line above must stay first; do not move the copyright
 # notice to the top.  See http://www.python.org/dev/peps/pep-0263/.
 #
-# Copyright 2009-2011 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 # pylint: disable-msg=F0401
@@ -2323,9 +2323,10 @@
             else:
                 merge_proposal = self.makeBranchMergeProposal(
                     registrant=sender)
-        return merge_proposal.createComment(
-            sender, subject, body, vote, vote_tag, parent,
-            _date_created=date_created)
+        with person_logged_in(sender):
+            return merge_proposal.createComment(
+                sender, subject, body, vote, vote_tag, parent,
+                _date_created=date_created)
 
     def makeCodeReviewVoteReference(self):
         bmp = removeSecurityProxy(self.makeBranchMergeProposal())