Merge ~pappacena/launchpad:comment-editing-api into launchpad:master

[Thiago F. Pappacena]

API to edit and delete comments for bug messages, answers and code review comments

The API for the message revision history (listing and deleting old revisions) will be done in a future MP, in order to keep the review shorter.

[Colin Watson (cjwatson)]

review: Approve

[Thiago F. Pappacena (pappacena)]

Addressed all the comments. I would like your opinion on the comment on xx-branchmergeproposal.txt, about deprecating an API attribute.

[Colin Watson (cjwatson)]

review: Approve

[Thiago F. Pappacena (pappacena)]

Pushed the requested change. This should be good to land now.

diff --git a/lib/lp/answers/browser/configure.zcml b/lib/lp/answers/browser/configure.zcml
index 46c7553..631e8b8 100644
--- a/lib/lp/answers/browser/configure.zcml
+++ b/lib/lp/answers/browser/configure.zcml
@@ -1,4 +1,4 @@
-<!-- Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
+<!-- Copyright 2009-2021 Canonical Ltd.  This software is licensed under the
      GNU Affero General Public License version 3 (see the file LICENSE).
 -->
 
@@ -278,6 +278,10 @@
     module=".question"
     classes="QuestionSetNavigation"
     />
+  <browser:navigation
+    module=".question"
+    classes="QuestionNavigation"
+    />
 
   <browser:url
     for="lp.answers.interfaces.questioncollection.IQuestionSet"
diff --git a/lib/lp/answers/browser/question.py b/lib/lp/answers/browser/question.py
index b039552..814f329 100644
--- a/lib/lp/answers/browser/question.py
+++ b/lib/lp/answers/browser/question.py
@@ -1,4 +1,4 @@
-# Copyright 2009-2020 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2021 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Question views."""
@@ -109,6 +109,7 @@ from lp.services.webapp import (
     Link,
     Navigation,
     NavigationMenu,
+    stepthrough,
     )
 from lp.services.webapp.authorization import check_permission
 from lp.services.webapp.breadcrumb import Breadcrumb
@@ -257,6 +258,23 @@ class QuestionSetNavigation(Navigation):
             canonical_url(question, self.request), status=301)
 
 
+class QuestionNavigation(Navigation):
+    """Navigation for the IQuestion."""
+
+    usedfor = IQuestion
+
+    @stepthrough('messages')
+    def traverse_messages(self, index):
+        try:
+            index = int(index) - 1
+        except ValueError:
+            return None
+        try:
+            return self.context.messages[index]
+        except IndexError:
+            return None
+
+
 class QuestionBreadcrumb(Breadcrumb):
     """Builds a breadcrumb for an `IQuestion`."""
 
diff --git a/lib/lp/answers/configure.zcml b/lib/lp/answers/configure.zcml
index 9735be8..a7eefcd 100644
--- a/lib/lp/answers/configure.zcml
+++ b/lib/lp/answers/configure.zcml
@@ -1,4 +1,4 @@
-<!-- Copyright 2009-2015 Canonical Ltd.  This software is licensed under the
+<!-- Copyright 2009-2021 Canonical Ltd.  This software is licensed under the
      GNU Affero General Public License version 3 (see the file LICENSE).
 -->
 
@@ -142,7 +142,9 @@
 
 
   <class class=".model.questionmessage.QuestionMessage">
-    <allow interface=".interfaces.questionmessage.IQuestionMessage"/>
+    <require permission="launchpad.Edit"
+            interface="lp.services.messages.interfaces.message.IMessageEdit" />
+    <allow interface=".interfaces.questionmessage.IQuestionMessageView"/>
   </class>
 
   <adapter
diff --git a/lib/lp/answers/interfaces/questionmessage.py b/lib/lp/answers/interfaces/questionmessage.py
index 176ddf8..dadd1d6 100644
--- a/lib/lp/answers/interfaces/questionmessage.py
+++ b/lib/lp/answers/interfaces/questionmessage.py
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2021 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Question message interface."""
@@ -26,15 +26,14 @@ from lp.answers.enums import (
     QuestionAction,
     QuestionStatus,
     )
-from lp.services.messages.interfaces.message import IMessage
-
+from lp.services.messages.interfaces.message import (
+    IMessage,
+    IMessageView,
+    )
 
-@exported_as_webservice_entry(as_of='devel')
-class IQuestionMessage(IMessage):
-    """A message part of a question.
 
-    It adds attributes to the IMessage interface.
-    """
+class IQuestionMessageView(IMessageView):
+    """Publicly visible attributes of a message part of a question."""
     # This is really an Object field with schema=IQuestion, but that
     # would create a circular dependency between IQuestion
     # and IQuestionMessage
@@ -69,3 +68,8 @@ class IQuestionMessage(IMessage):
         description=_("Whether or not the message is visible."),
         readonly=True),
         as_of="devel")
+
+
+@exported_as_webservice_entry(as_of='devel')
+class IQuestionMessage(IQuestionMessageView, IMessage):
+    """A message part of a question."""
diff --git a/lib/lp/answers/model/questionmessage.py b/lib/lp/answers/model/questionmessage.py
index 3acf257..3fe7c56 100644
--- a/lib/lp/answers/model/questionmessage.py
+++ b/lib/lp/answers/model/questionmessage.py
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2021 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """SQLBase implementation of IQuestionMessage."""
diff --git a/lib/lp/answers/stories/webservice.txt b/lib/lp/answers/stories/webservice.txt
index 852e915..8ab277c 100644
--- a/lib/lp/answers/stories/webservice.txt
+++ b/lib/lp/answers/stories/webservice.txt
@@ -234,6 +234,8 @@ that indicate how the message changed the question.
     bug_attachments_collection_link: '...'
     content: 'This is the answer'
     date_created: '20...+00:00'
+    date_deleted: None
+    date_last_edited: None
     index: 1
     new_status: 'Answered'
     owner_link: 'http://api.launchpad.test/devel/~contact'
diff --git a/lib/lp/answers/tests/test_question_workflow.py b/lib/lp/answers/tests/test_question_workflow.py
index 6450d61..66fa81b 100644
--- a/lib/lp/answers/tests/test_question_workflow.py
+++ b/lib/lp/answers/tests/test_question_workflow.py
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2021 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Test the question workflow methods.
@@ -53,6 +53,7 @@ from lp.testing import (
     ANONYMOUS,
     login,
     login_person,
+    person_logged_in,
     TestCase,
     )
 from lp.testing.fixture import ZopeEventHandlerFixture
@@ -238,7 +239,8 @@ class BaseAnswerTrackerWorkflowTestCase(TestCase):
         It also verifies that the question status, datelastquery (or
         datelastresponse) were updated to reflect the time of the message.
         """
-        self.assertTrue(verifyObject(IQuestionMessage, message))
+        with person_logged_in(message.owner):
+            self.assertTrue(verifyObject(IQuestionMessage, message))
 
         self.assertEqual("Re: Help!", message.subject)
         self.assertEqual(expected_owner, message.owner)
diff --git a/lib/lp/bugs/browser/bugcomment.py b/lib/lp/bugs/browser/bugcomment.py
index 2b61552..e40967f 100644
--- a/lib/lp/bugs/browser/bugcomment.py
+++ b/lib/lp/bugs/browser/bugcomment.py
@@ -1,4 +1,4 @@
-# Copyright 2006-2020 Canonical Ltd.  This software is licensed under the
+# Copyright 2006-2021 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Bug comment browser view classes."""
@@ -72,7 +72,10 @@ def build_comments_from_chunks(
         cache = get_property_cache(message)
         if getattr(cache, 'chunks', None) is None:
             cache.chunks = []
-        cache.chunks.append(removeSecurityProxy(chunk))
+        # Soft-deleted messages will have None chunk here. Skip cache
+        # filling in this case.
+        if chunk is not None:
+            cache.chunks.append(removeSecurityProxy(chunk))
         bug_comment = comments.get(message.id)
         if bug_comment is None:
             if bugmessage.index == 0 and hide_first:
diff --git a/lib/lp/bugs/configure.zcml b/lib/lp/bugs/configure.zcml
index 73b99e8..b2a54ac 100644
--- a/lib/lp/bugs/configure.zcml
+++ b/lib/lp/bugs/configure.zcml
@@ -1,4 +1,4 @@
-<!-- Copyright 2009-2020 Canonical Ltd.  This software is licensed under the
+<!-- Copyright 2009-2021 Canonical Ltd.  This software is licensed under the
      GNU Affero General Public License version 3 (see the file LICENSE).
 -->
 
@@ -773,7 +773,9 @@
     <class
         class="lp.bugs.model.bugmessage.BugMessage">
         <allow
-            interface="lp.bugs.interfaces.bugmessage.IBugMessage"/>
+            interface="lp.bugs.interfaces.bugmessage.IBugMessageView"/>
+        <require permission="launchpad.Edit"
+            interface="lp.services.messages.interfaces.message.IMessageEdit" />
         <require
             permission="launchpad.InternalScriptsOnly"
             set_attributes="remote_comment_id bugwatch index"/>
diff --git a/lib/lp/bugs/interfaces/bugmessage.py b/lib/lp/bugs/interfaces/bugmessage.py
index 98c18f3..15b3373 100644
--- a/lib/lp/bugs/interfaces/bugmessage.py
+++ b/lib/lp/bugs/interfaces/bugmessage.py
@@ -1,4 +1,4 @@
-# Copyright 2004-2020 Canonical Ltd.  This software is licensed under the
+# Copyright 2004-2021 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Bug message interfaces."""
@@ -31,11 +31,14 @@ from lp.bugs.interfaces.hasbug import IHasBug
 from lp.registry.interfaces.person import IPerson
 from lp.services.comments.interfaces.conversation import IComment
 from lp.services.fields import Title
-from lp.services.messages.interfaces.message import IMessage
+from lp.services.messages.interfaces.message import (
+    IMessage,
+    IMessageView,
+    )
 
 
-class IBugMessage(IHasBug):
-    """A link between a bug and a message."""
+class IBugMessageView(IMessageView, IHasBug):
+    """Public attributes for a link between a bug and a message."""
 
     bug = Object(schema=IBug, title=u"The bug.")
     # The index field is being populated in the DB; once complete it will be
@@ -57,6 +60,10 @@ class IBugMessage(IHasBug):
         title=u"The Message owner mirrored from the message.", readonly=True)
 
 
+class IBugMessage(IBugMessageView, IMessage):
+    """A link between a bug and a message."""
+
+
 class IBugMessageSet(Interface):
     """The set of all IBugMessages."""
 
diff --git a/lib/lp/bugs/model/bug.py b/lib/lp/bugs/model/bug.py
index 8f1480c..c1cc4c8 100644
--- a/lib/lp/bugs/model/bug.py
+++ b/lib/lp/bugs/model/bug.py
@@ -1577,8 +1577,12 @@ class Bug(SQLBase, InformationTypeMixin):
         # We expect:
         # 1 bugmessage -> 1 message -> small N chunks. For now, using a wide
         # query seems fine as we have to join out from bugmessage anyway.
-        result = Store.of(self).find((BugMessage, Message, MessageChunk),
-            Message.id == MessageChunk.messageID,
+        # Since "soft-deleted" messages will have 0 chunks, we should use
+        # left join here.
+        message_join = LeftJoin(
+            Message, MessageChunk, Message.id == MessageChunk.messageID)
+        query = Store.of(self).using(BugMessage, message_join)
+        result = query.find((BugMessage, Message, MessageChunk),
             BugMessage.message_id == Message.id,
             BugMessage.bug == self.id, *ranges)
         result.order_by(BugMessage.index, MessageChunk.sequence)
diff --git a/lib/lp/bugs/model/bugmessage.py b/lib/lp/bugs/model/bugmessage.py
index 97c3015..b8e7291 100644
--- a/lib/lp/bugs/model/bugmessage.py
+++ b/lib/lp/bugs/model/bugmessage.py
@@ -1,4 +1,4 @@
-# Copyright 2009-2020 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2021 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __metaclass__ = type
@@ -6,6 +6,7 @@ __all__ = ['BugMessage', 'BugMessageSet']
 
 from email.utils import make_msgid
 
+from lazr.delegates import delegate_to
 import six
 from storm.properties import (
     Int,
@@ -22,6 +23,7 @@ from lp.bugs.interfaces.bugmessage import (
 from lp.registry.interfaces.person import validate_public_person
 from lp.services.database.interfaces import IStore
 from lp.services.database.stormbase import StormBase
+from lp.services.messages.interfaces.message import IMessage
 from lp.services.messages.model.message import (
     Message,
     MessageChunk,
@@ -29,6 +31,7 @@ from lp.services.messages.model.message import (
 
 
 @implementer(IBugMessage)
+@delegate_to(IMessage, context='message')
 class BugMessage(StormBase):
     """A table linking bugs and messages."""
 
diff --git a/lib/lp/bugs/stories/webservice/xx-bug.txt b/lib/lp/bugs/stories/webservice/xx-bug.txt
index 6121737..77ad62d 100644
--- a/lib/lp/bugs/stories/webservice/xx-bug.txt
+++ b/lib/lp/bugs/stories/webservice/xx-bug.txt
@@ -234,6 +234,8 @@ Each bug has a collection of messages.
      'http://.../firefox/+bug/5/comments/0/bug_attachments'
     content: 'All ways of downloading firefox should provide...'
     date_created: '2005-01-14T17:27:03.702622+00:00'
+    date_deleted: None
+    date_last_edited: None
     owner_link: 'http://.../~name12'
     parent_link: None
     resource_type_link: 'http://.../#message'
@@ -1347,6 +1349,8 @@ attachment. This is where our comment is recorded.
     bug_attachments_collection_link: 'http://.../firefox/+bug/1/comments/2/bug_attachments'
     content: 'The numbers you asked for.'
     date_created: '...'
+    date_deleted: None
+    date_last_edited: None
     owner_link: 'http://.../~salgado'
     parent_link: None
     resource_type_link: 'http://.../#message'
diff --git a/lib/lp/code/browser/codereviewcomment.py b/lib/lp/code/browser/codereviewcomment.py
index 24af2f8..0d0496d 100644
--- a/lib/lp/code/browser/codereviewcomment.py
+++ b/lib/lp/code/browser/codereviewcomment.py
@@ -1,4 +1,4 @@
-# Copyright 2009-2018 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2021 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __metaclass__ = type
@@ -23,7 +23,10 @@ from zope.interface import (
     implementer,
     Interface,
     )
-from zope.schema import Text
+from zope.schema import (
+    Object,
+    Text,
+    )
 
 from lp import _
 from lp.app.browser.launchpadform import (
@@ -40,6 +43,10 @@ from lp.services.comments.browser.messagecomment import MessageComment
 from lp.services.comments.interfaces.conversation import IComment
 from lp.services.config import config
 from lp.services.librarian.interfaces import ILibraryFileAlias
+from lp.services.messages.interfaces.message import (
+    IMessage,
+    IMessageEdit,
+    )
 from lp.services.propertycache import (
     cachedproperty,
     get_property_cache,
@@ -55,10 +62,12 @@ from lp.services.webapp.interfaces import ILaunchBag
 
 class ICodeReviewDisplayComment(IComment, ICodeReviewComment):
     """Marker interface for displaying code review comments."""
+    message = Object(schema=IMessage, title=_('The message.'))
 
 
 @implementer(ICodeReviewDisplayComment)
 @delegate_to(ICodeReviewComment, context='comment')
+@delegate_to(IMessageEdit, context='message')
 class CodeReviewDisplayComment(MessageComment):
     """A code review comment or activity or both.
 
diff --git a/lib/lp/code/browser/tests/test_codereviewcomment.py b/lib/lp/code/browser/tests/test_codereviewcomment.py
index 799312b..e74e310 100644
--- a/lib/lp/code/browser/tests/test_codereviewcomment.py
+++ b/lib/lp/code/browser/tests/test_codereviewcomment.py
@@ -1,4 +1,4 @@
-# Copyright 2009-2018 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2021 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Unit tests for CodeReviewComments."""
@@ -56,7 +56,8 @@ class TestCodeReviewComments(TestCaseWithFactory):
 
         display_comment = CodeReviewDisplayComment(comment)
 
-        verifyObject(ICodeReviewDisplayComment, display_comment)
+        with person_logged_in(comment.owner):
+            verifyObject(ICodeReviewDisplayComment, display_comment)
 
     def test_extra_css_classes_visibility(self):
         author = self.factory.makePerson()
diff --git a/lib/lp/code/configure.zcml b/lib/lp/code/configure.zcml
index 61c18c1..a373286 100644
--- a/lib/lp/code/configure.zcml
+++ b/lib/lp/code/configure.zcml
@@ -1,4 +1,4 @@
-<!-- Copyright 2009-2020 Canonical Ltd.  This software is licensed under the
+<!-- Copyright 2009-2021 Canonical Ltd.  This software is licensed under the
      GNU Affero General Public License version 3 (see the file LICENSE).
 -->
 
@@ -548,7 +548,9 @@
   <!-- CodeReviewComment -->
 
   <class class="lp.code.model.codereviewcomment.CodeReviewComment">
-    <allow interface="lp.code.interfaces.codereviewcomment.ICodeReviewComment"/>
+    <require permission="launchpad.Edit"
+             interface="lp.services.messages.interfaces.message.IMessageEdit" />
+    <allow interface="lp.code.interfaces.codereviewcomment.ICodeReviewCommentView"/>
     <allow interface="lp.code.interfaces.branchtarget.IHasBranchTarget"/>
   </class>
   <subscriber
diff --git a/lib/lp/code/interfaces/codereviewcomment.py b/lib/lp/code/interfaces/codereviewcomment.py
index f17e9a9..d32fcdd 100644
--- a/lib/lp/code/interfaces/codereviewcomment.py
+++ b/lib/lp/code/interfaces/codereviewcomment.py
@@ -1,4 +1,4 @@
-# Copyright 2009-2018 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2021 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """CodeReviewComment interfaces."""
@@ -28,12 +28,15 @@ from lp import _
 from lp.code.enums import CodeReviewVote
 from lp.code.interfaces.branchmergeproposal import IBranchMergeProposal
 from lp.registry.interfaces.person import IPerson
-from lp.services.messages.interfaces.message import IMessage
+from lp.services.messages.interfaces.message import (
+    IMessage,
+    IMessageCommon,
+    IMessageEdit,
+    )
 
 
-@exported_as_webservice_entry()
-class ICodeReviewComment(Interface):
-    """A link between a merge proposal and a message."""
+class ICodeReviewCommentView(IMessageCommon):
+    """Globally visible attributes of ICodeReviewComment."""
 
     id = exported(
         Int(
@@ -69,7 +72,7 @@ class ICodeReviewComment(Interface):
 
     message_body = exported(
         TextLine(
-            title=_('The body of the code review message.'),
+            title=_('Deprecated. Use "content" attribute instead.'),
             readonly=True))
 
     def getAttachments():
@@ -99,6 +102,11 @@ class ICodeReviewComment(Interface):
         """
 
 
+@exported_as_webservice_entry()
+class ICodeReviewComment(ICodeReviewCommentView, IMessageEdit):
+    """A link between a merge proposal and a message."""
+
+
 class ICodeReviewCommentDeletion(Interface):
     """This interface provides deletion of CodeReviewComments.
 
diff --git a/lib/lp/code/model/codereviewcomment.py b/lib/lp/code/model/codereviewcomment.py
index 68b9d78..53e8a84 100644
--- a/lib/lp/code/model/codereviewcomment.py
+++ b/lib/lp/code/model/codereviewcomment.py
@@ -1,4 +1,4 @@
-# Copyright 2009-2018 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2021 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """The database implementation class for CodeReviewComment."""
@@ -10,6 +10,7 @@ __all__ = [
 
 from textwrap import TextWrapper
 
+from lazr.delegates import delegate_to
 from storm.locals import (
     Int,
     Reference,
@@ -27,6 +28,10 @@ from lp.code.interfaces.codereviewcomment import (
 from lp.services.database.enumcol import DBEnum
 from lp.services.database.stormbase import StormBase
 from lp.services.mail.signedmessage import signed_message_from_bytes
+from lp.services.messages.interfaces.message import (
+    IMessageCommon,
+    IMessageEdit,
+    )
 
 
 def quote_text_as_email(text, width=80):
@@ -61,7 +66,9 @@ def quote_text_as_email(text, width=80):
     return '\n'.join(result)
 
 
-@implementer(ICodeReviewComment, ICodeReviewCommentDeletion, IHasBranchTarget)
+@implementer(ICodeReviewComment, ICodeReviewCommentDeletion, IHasBranchTarget,
+             IMessageCommon, IMessageEdit)
+@delegate_to(IMessageCommon, IMessageEdit, context='message')
 class CodeReviewComment(StormBase):
     """A table linking branch merge proposals and messages."""
 
diff --git a/lib/lp/code/stories/webservice/xx-branchmergeproposal.txt b/lib/lp/code/stories/webservice/xx-branchmergeproposal.txt
index 32f0373..58b86e7 100644
--- a/lib/lp/code/stories/webservice/xx-branchmergeproposal.txt
+++ b/lib/lp/code/stories/webservice/xx-branchmergeproposal.txt
@@ -198,9 +198,13 @@ The comments on a branch merge proposal are exposed through the API.
     as_quoted_email: '> This is great work'
     author_link: 'http://api.launchpad.test/devel/~...'
     branch_merge_proposal_link: 'http://.../~source/fooix/fix-it/+merge/...'
+    content: 'This is great work'
     date_created: '...'
+    date_deleted: None
+    date_last_edited: None
     id: ...
     message_body: 'This is great work'
+    owner_link: 'http://...'
     resource_type_link: 'http://.../#code_review_comment'
     self_link: 'http://.../~source/fooix/fix-it/+merge/.../comments/...'
     title: 'Comment on proposed merge of lp://dev/~source/fooix/fix-it into lp://dev/~target/fooix/trunk'
@@ -216,9 +220,13 @@ The comments on a branch merge proposal are exposed through the API.
     as_quoted_email: '> This is mediocre work.'
     author_link: 'http://api.launchpad.test/devel/~...'
     branch_merge_proposal_link: 'http://.../~source/fooix/fix-it/+merge/...'
+    content: 'This is mediocre work.'
     date_created: '...'
+    date_deleted: None
+    date_last_edited: None
     id: ...
     message_body: 'This is mediocre work.'
+    owner_link: 'http://...'
     resource_type_link: 'http://.../#code_review_comment'
     self_link: 'http://.../~source/fooix/fix-it/+merge/.../comments/...'
     title: ...
@@ -290,9 +298,13 @@ Now the code review should be made.
     as_quoted_email: '> This is great work'
     author_link: 'http://api.launchpad.test/devel/~...'
     branch_merge_proposal_link: 'http://.../~source/fooix/fix-it/+merge/...'
+    content: 'This is great work'
     date_created: '...'
+    date_deleted: None
+    date_last_edited: None
     id: ...
     message_body: 'This is great work'
+    owner_link: 'http://...'
     resource_type_link: 'http://.../#code_review_comment'
     self_link: 'http://.../~source/fooix/fix-it/+merge/.../comments/...'
     title: ...
diff --git a/lib/lp/services/messages/browser/message.py b/lib/lp/services/messages/browser/message.py
index 3b5a3c4..06800e6 100644
--- a/lib/lp/services/messages/browser/message.py
+++ b/lib/lp/services/messages/browser/message.py
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2021 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Message related view classes."""
@@ -18,7 +18,7 @@ class QuestionMessageCanonicalUrlData:
 
     def __init__(self, question, message):
         self.inside = question
-        self.path = "messages/%d" % list(question.messages).index(message)
+        self.path = "messages/%d" % message.display_index
 
 
 @implementer(ICanonicalUrlData)
@@ -45,14 +45,28 @@ class IndexedBugMessageCanonicalUrlData:
         self.path = "comments/%d" % message.index
 
 
+@implementer(ICanonicalUrlData)
+class CodeReviewCommentCanonicalUrlData:
+    """An optimized bug message canonical_url implementation.
+    """
+    rootsite = 'code'
+
+    def __init__(self, message):
+        self.inside = message.branch_merge_proposal
+        self.path = "comments/%d" % message.id
+
+
 def message_to_canonical_url_data(message):
     """This factory creates `ICanonicalUrlData` for Message."""
     # Circular imports
     from lp.answers.interfaces.questionmessage import IQuestionMessage
+    from lp.code.interfaces.codereviewcomment import ICodeReviewComment
     if IIndexedMessage.providedBy(message):
         return IndexedBugMessageCanonicalUrlData(message)
     elif IQuestionMessage.providedBy(message):
         return QuestionMessageCanonicalUrlData(message.question, message)
+    elif ICodeReviewComment.providedBy(message):
+        return CodeReviewCommentCanonicalUrlData(message)
     else:
         if message.bugs.count() == 0:
         # Will result in a ComponentLookupError
diff --git a/lib/lp/services/messages/interfaces/message.py b/lib/lp/services/messages/interfaces/message.py
index 70d50b7..6d4e186 100644
--- a/lib/lp/services/messages/interfaces/message.py
+++ b/lib/lp/services/messages/interfaces/message.py
@@ -8,7 +8,10 @@ __all__ = [
     'IIndexedMessage',
     'IMessage',
     'IMessageChunk',
+    'IMessageCommon',
+    'IMessageEdit',
     'IMessageSet',
+    'IMessageView',
     'IUserToUserEmail',
     'IndexedMessage',
     'InvalidEmailMessage',
@@ -21,9 +24,11 @@ from lazr.delegates import delegate_to
 from lazr.restful.declarations import (
     accessor_for,
     export_read_operation,
+    export_write_operation,
     exported,
     exported_as_webservice_entry,
     operation_for_version,
+    operation_parameters,
     )
 from lazr.restful.fields import (
     CollectionField,
@@ -51,44 +56,62 @@ from lp.services.webservice.apihelpers import patch_reference_property
 
 class IMessageEdit(Interface):
 
+    @export_write_operation()
+    @operation_parameters(
+        new_content=TextLine(
+            title=_("Message content"),
+            description=_("The new message content string"),
+            required=True))
+    @operation_for_version("devel")
     def editContent(new_content):
         """Edit the content of this message, generating a new message
         revision with the old content.
         """
 
+    @export_write_operation()
+    @operation_for_version("devel")
     def deleteContent():
         """Deletes this message content."""
 
 
-class IMessageView(Interface):
-    """Public attributes for message.
-
-    This is like an email (RFC822) message, though it could be created through
-    the web as well.
-    """
+class IMessageCommon(Interface):
+    """Common public attributes for every IMessage implementation."""
 
     id = Int(title=_('ID'), required=True, readonly=True)
+
+    chunks = Attribute(_('Message pieces'))
+    text_contents = exported(
+        Text(title=_('All the text/plain chunks joined together as a '
+                     'unicode string.'), readonly=True),
+        exported_as='content')
+    owner = exported(
+        Reference(title=_('Person'), schema=Interface,
+                  required=False, readonly=True))
+
+    revisions = Attribute(_('Message revision history'))
     datecreated = exported(
         Datetime(title=_('Date Created'), required=True, readonly=True),
         exported_as='date_created')
+    date_last_edited = exported(Datetime(
 
-    date_last_edited = Datetime(
         title=_('When this message was last edited'), required=False,
-        readonly=True)
-
-    date_deleted = Datetime(
+        readonly=True))
+    date_deleted = exported(Datetime(
         title=_('When this message was deleted'), required=False,
-        readonly=True)
+        readonly=True))
+
+
+class IMessageView(IMessageCommon):
+    """Public attributes for message.
+
+    This is like an email (RFC822) message, though it could be created through
+    the web as well.
+    """
 
     subject = exported(
         TextLine(title=_('Subject'), required=True, readonly=True))
 
-    # XXX flacoste 2006-09-08: This attribute is only used for the
-    # add form used by MessageAddView.
     content = Text(title=_("Message"), required=True, readonly=True)
-    owner = exported(
-        Reference(title=_('Person'), schema=Interface,
-                  required=False, readonly=True))
 
     # Schema is really IMessage, but this cannot be declared here. It's
     # fixed below after the IMessage definition is complete.
@@ -104,15 +127,6 @@ class IMessageView(Interface):
         title=_('Bug List'),
         value_type=Reference(schema=Interface))  # Redefined in bug.py
 
-    chunks = Attribute(_('Message pieces'))
-
-    revisions = Attribute(_('Message revision history'))
-
-    text_contents = exported(
-        Text(title=_('All the text/plain chunks joined together as a '
-                     'unicode string.')),
-        exported_as='content')
-
     title = TextLine(
         title=_('The message title, usually just the subject.'),
         readonly=True)
diff --git a/lib/lp/services/messages/model/message.py b/lib/lp/services/messages/model/message.py
index be685d6..d20ca2f 100644
--- a/lib/lp/services/messages/model/message.py
+++ b/lib/lp/services/messages/model/message.py
@@ -40,7 +40,6 @@ from sqlobject import (
 from storm.locals import (
     And,
     DateTime,
-    Desc,
     Int,
     Max,
     Reference,
@@ -181,7 +180,7 @@ class Message(SQLBase):
         return list(Store.of(self).find(
             MessageRevision,
             MessageRevision.message == self
-        ).order_by(Desc(MessageRevision.revision)))
+        ).order_by(MessageRevision.revision))
 
     def editContent(self, new_content):
         """See `IMessage`."""
@@ -235,7 +234,6 @@ class Message(SQLBase):
         del get_property_cache(self).text_contents
         del get_property_cache(self).chunks
         del get_property_cache(self).revisions
-        return rev
 
     def deleteContent(self):
         """See `IMessage`."""
diff --git a/lib/lp/services/messages/tests/test_message.py b/lib/lp/services/messages/tests/test_message.py
index 9c2bbc7..070bc91 100644
--- a/lib/lp/services/messages/tests/test_message.py
+++ b/lib/lp/services/messages/tests/test_message.py
@@ -13,6 +13,7 @@ from email.utils import (
     )
 
 import six
+from testscenarios import WithScenarios
 from testtools.matchers import (
     Equals,
     Is,
@@ -20,8 +21,13 @@ from testtools.matchers import (
     )
 import transaction
 from zope.security.interfaces import Unauthorized
-from zope.security.proxy import ProxyFactory
+from zope.security.proxy import (
+    ProxyFactory,
+    removeSecurityProxy,
+    )
 
+from lp.bugs.interfaces.bugmessage import IBugMessage
+from lp.bugs.model.bugmessage import BugMessage
 from lp.services.compat import message_as_bytes
 from lp.services.database.interfaces import IStore
 from lp.services.database.sqlbase import get_transaction_timestamp
@@ -29,8 +35,12 @@ from lp.services.messages.model.message import (
     MessageChunk,
     MessageSet,
     )
+from lp.services.webapp.interfaces import OAuthPermission
 from lp.testing import (
+    admin_logged_in,
+    api_url,
     login,
+    login_person,
     person_logged_in,
     TestCaseWithFactory,
     )
@@ -38,6 +48,7 @@ from lp.testing.layers import (
     DatabaseFunctionalLayer,
     LaunchpadFunctionalLayer,
     )
+from lp.testing.pages import webservice_for_person
 
 
 class TestMessageSet(TestCaseWithFactory):
@@ -187,16 +198,50 @@ class TestMessageSet(TestCaseWithFactory):
         self.assertEqual(self.high_characters.decode('latin-1'), result)
 
 
-class TestMessageEditing(TestCaseWithFactory):
+class MessageTypeScenariosMixin(WithScenarios):
+
+    scenarios = [
+        ("bug", {"message_type": "bug"}),
+        ("question", {"message_type": "question"}),
+        ("MP comment", {"message_type": "mp"})
+        ]
+
+    def setUp(self):
+        super(MessageTypeScenariosMixin, self).setUp()
+        self.person = self.factory.makePerson()
+        login_person(self.person)
+
+    def makeMessage(self, content=None, **kwargs):
+        owner = kwargs.pop('owner', self.person)
+        if self.message_type == "bug":
+            msg = self.factory.makeBugComment(
+                owner=owner, body=content, **kwargs)
+            return ProxyFactory(IStore(BugMessage).find(
+                BugMessage, BugMessage.message == msg).one())
+        elif self.message_type == "question":
+            question = self.factory.makeQuestion()
+            return question.giveAnswer(owner, content)
+        elif self.message_type == "mp":
+            return self.factory.makeCodeReviewComment(
+                sender=owner, body=content)
+
+
+class TestMessageEditing(MessageTypeScenariosMixin, TestCaseWithFactory):
     """Test editing scenarios for Message objects."""
 
     layer = DatabaseFunctionalLayer
 
-    def makeMessage(self, owner=None, content=None):
-        if owner is None:
-            owner = self.factory.makePerson()
-        msg = self.factory.makeMessage(owner=owner, content=content)
-        return ProxyFactory(msg)
+    def assertIsMessageHistory(
+            self, msg_history, msg, rev, created_at, content, deleted_at=None):
+        """Asserts that `msg_history` is a message history of
+        `msg` with the given extra info.
+        """
+        self.assertThat(msg_history, MatchesStructure(
+            content=Equals(content),
+            revision=Equals(rev),
+            message=Equals(removeSecurityProxy(msg).message),
+            date_created=Equals(created_at),
+            date_deleted=Equals(deleted_at)))
 
     def test_non_owner_cannot_edit_message(self):
         msg = self.makeMessage()
@@ -211,12 +256,9 @@ class TestMessageEditing(TestCaseWithFactory):
             msg.editContent("This is the new content")
         self.assertEqual("This is the new content", msg.text_contents)
         self.assertEqual(1, len(msg.revisions))
-        self.assertThat(msg.revisions[0], MatchesStructure(
-            content=Equals("initial content"),
-            revision=Equals(1),
-            message=Equals(msg),
-            date_created=Equals(msg.datecreated),
-            date_deleted=Is(None)))
+        self.assertIsMessageHistory(
+            msg.revisions[0], msg, rev=1,
+            created_at=msg.datecreated, content="initial content")
 
     def test_multiple_edits_revisions(self):
         owner = self.factory.makePerson()
@@ -226,47 +268,43 @@ class TestMessageEditing(TestCaseWithFactory):
             first_edit_date = msg.date_last_edited
         self.assertEqual("first edit", msg.text_contents)
         self.assertEqual(1, len(msg.revisions))
-        self.assertThat(msg.revisions[0], MatchesStructure(
-            content=Equals("initial content"),
-            revision=Equals(1),
-            message=Equals(msg),
-            date_created=Equals(msg.datecreated),
-            date_deleted=Is(None)))
+        self.assertIsMessageHistory(
+            msg.revisions[0], msg, rev=1,
+            content="initial content", created_at=msg.datecreated)
 
         with person_logged_in(owner):
             msg.editContent("final form")
         self.assertEqual("final form", msg.text_contents)
         self.assertEqual(2, len(msg.revisions))
-        self.assertThat(msg.revisions[0], MatchesStructure(
-            content=Equals("first edit"),
-            revision=Equals(2),
-            message=Equals(msg),
-            date_created=Equals(first_edit_date),
-            date_deleted=Is(None)))
-        self.assertThat(msg.revisions[1], MatchesStructure(
-            content=Equals("initial content"),
-            revision=Equals(1),
-            message=Equals(msg),
-            date_created=Equals(msg.datecreated),
-            date_deleted=Is(None)))
+
+        self.assertIsMessageHistory(
+            msg.revisions[0], msg, rev=1,
+            content="initial content", created_at=msg.datecreated)
+        self.assertIsMessageHistory(
+            msg.revisions[1], msg, rev=2,
+            content="first edit", created_at=first_edit_date)
 
     def test_edit_message_with_blobs(self):
         # Messages with blobs should keep the blobs untouched when the
         # content is edited.
         owner = self.factory.makePerson()
         msg = self.makeMessage(owner=owner, content="initial content")
+        # The IMessage object (not the delegate one).
+        raw_msg = removeSecurityProxy(msg).message
+
         files = [self.factory.makeLibraryFileAlias(db_only=True)
                  for _ in range(2)]
         store = IStore(msg)
         for seq, blob in enumerate(files):
-            store.add(MessageChunk(message=msg, sequence=seq + 2, blob=blob))
+            store.add(MessageChunk(
+                message=raw_msg, sequence=seq + 2, blob=blob))
 
         with person_logged_in(owner):
             msg.editContent("final form")
         self.assertThat(msg.revisions[0], MatchesStructure(
             content=Equals("initial content"),
             revision=Equals(1),
-            message=Equals(msg),
+            message=Equals(raw_msg),
             date_created=Equals(msg.datecreated),
             date_deleted=Is(None)))
 
@@ -307,3 +345,73 @@ class TestMessageEditing(TestCaseWithFactory):
         self.assertEqual(
             get_transaction_timestamp(IStore(msg)), msg.date_deleted)
         self.assertEqual(0, len(msg.revisions))
+
+
+class TestMessageEditingAPI(MessageTypeScenariosMixin, TestCaseWithFactory):
+    """Test editing scenarios for Message editing API."""
+
+    layer = DatabaseFunctionalLayer
+
+    def getWebservice(self, person):
+        return webservice_for_person(
+            person, permission=OAuthPermission.WRITE_PUBLIC,
+            default_api_version="devel")
+
+    def getMessageAPIURL(self, msg):
+        with admin_logged_in():
+            if IBugMessage.providedBy(msg):
+                # BugMessage has a special URL mapping that uses the
+                # IMessage object itself.
+                return api_url(msg.message)
+            else:
+                return api_url(msg)
+
+    def test_edit_message(self):
+        msg = self.makeMessage(content="initial content")
+        ws = self.getWebservice(self.person)
+        url = self.getMessageAPIURL(msg)
+        response = ws.named_post(
+            url, 'editContent', new_content="the new content")
+        self.assertEqual(200, response.status)
+
+        edited_obj = ws.get(url).jsonBody()
+        self.assertEqual("the new content", edited_obj['content'])
+        self.assertIsNone(edited_obj["date_deleted"])
+        self.assertIsNotNone(edited_obj["date_last_edited"])
+
+    def test_edit_message_permission_denied_for_non_owner(self):
+        msg = self.makeMessage(content="initial content")
+        ws = self.getWebservice(self.factory.makePerson())
+        url = self.getMessageAPIURL(msg)
+        response = ws.named_post(
+            url, 'editContent', new_content="the new content")
+        self.assertEqual(401, response.status)
+
+        edited_obj = ws.get(url).jsonBody()
+        self.assertEqual("initial content", edited_obj['content'])
+        self.assertIsNone(edited_obj["date_deleted"])
+        self.assertIsNone(edited_obj["date_last_edited"])
+
+    def test_delete_message(self):
+        msg = self.makeMessage(content="initial content")
+        ws = self.getWebservice(self.person)
+        url = self.getMessageAPIURL(msg)
+
+        response = ws.named_post(url, 'deleteContent')
+        self.assertEqual(200, response.status)
+
+        deleted_obj = ws.get(url).jsonBody()
+        self.assertEqual("", deleted_obj['content'])
+        self.assertIsNotNone(deleted_obj['date_deleted'])
+
+    def test_delete_message_permission_denied_for_non_owner(self):
+        msg = self.makeMessage(content="initial content")
+        ws = self.getWebservice(self.factory.makePerson())
+        url = self.getMessageAPIURL(msg)
+
+        response = ws.named_post(url, 'deleteContent')
+        self.assertEqual(401, response.status)
+
+        obj = ws.get(url).jsonBody()
+        self.assertEqual("initial content", obj['content'])
+        self.assertIsNone(obj['date_deleted'])