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

Proposed by Thiago F. Pappacena
Status: Merged
Approved by: Thiago F. Pappacena
Approved revision: 386583b9a35cbcde4db12e0504755e1d52167479
Merge reported by: Otto Co-Pilot
Merged at revision: not available
Proposed branch: ~pappacena/launchpad:comment-editing-api
Merge into: launchpad:master
Prerequisite: ~pappacena/launchpad:comment-editing-model
Diff against target: 1015 lines (+331/-103)
23 files modified
lib/lp/answers/browser/configure.zcml (+5/-1)
lib/lp/answers/browser/question.py (+19/-1)
lib/lp/answers/configure.zcml (+4/-2)
lib/lp/answers/interfaces/questionmessage.py (+12/-8)
lib/lp/answers/model/questionmessage.py (+1/-1)
lib/lp/answers/stories/webservice.txt (+2/-0)
lib/lp/answers/tests/test_question_workflow.py (+4/-2)
lib/lp/bugs/browser/bugcomment.py (+5/-2)
lib/lp/bugs/configure.zcml (+4/-2)
lib/lp/bugs/interfaces/bugmessage.py (+11/-4)
lib/lp/bugs/model/bug.py (+6/-2)
lib/lp/bugs/model/bugmessage.py (+4/-1)
lib/lp/bugs/stories/webservice/xx-bug.txt (+4/-0)
lib/lp/code/browser/codereviewcomment.py (+11/-2)
lib/lp/code/browser/tests/test_codereviewcomment.py (+3/-2)
lib/lp/code/configure.zcml (+4/-2)
lib/lp/code/interfaces/codereviewcomment.py (+14/-6)
lib/lp/code/model/codereviewcomment.py (+9/-2)
lib/lp/code/stories/webservice/xx-branchmergeproposal.txt (+12/-0)
lib/lp/services/messages/browser/message.py (+16/-2)
lib/lp/services/messages/interfaces/message.py (+39/-25)
lib/lp/services/messages/model/message.py (+1/-3)
lib/lp/services/messages/tests/test_message.py (+141/-33)
Reviewer Review Type Date Requested Status
Colin Watson Approve
Review via email: mp+402211@code.launchpad.net

Commit message

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

Description of the change

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.

To post a comment you must log in.
Revision history for this message
Colin Watson (cjwatson) :
review: Approve
Revision history for this message
Thiago F. Pappacena (pappacena) wrote :

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

Revision history for this message
Colin Watson (cjwatson) :
review: Approve
Revision history for this message
Thiago F. Pappacena (pappacena) wrote :

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

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/lib/lp/answers/browser/configure.zcml b/lib/lp/answers/browser/configure.zcml
2index 46c7553..631e8b8 100644
3--- a/lib/lp/answers/browser/configure.zcml
4+++ b/lib/lp/answers/browser/configure.zcml
5@@ -1,4 +1,4 @@
6-<!-- Copyright 2009-2012 Canonical Ltd. This software is licensed under the
7+<!-- Copyright 2009-2021 Canonical Ltd. This software is licensed under the
8 GNU Affero General Public License version 3 (see the file LICENSE).
9 -->
10
11@@ -278,6 +278,10 @@
12 module=".question"
13 classes="QuestionSetNavigation"
14 />
15+ <browser:navigation
16+ module=".question"
17+ classes="QuestionNavigation"
18+ />
19
20 <browser:url
21 for="lp.answers.interfaces.questioncollection.IQuestionSet"
22diff --git a/lib/lp/answers/browser/question.py b/lib/lp/answers/browser/question.py
23index b039552..814f329 100644
24--- a/lib/lp/answers/browser/question.py
25+++ b/lib/lp/answers/browser/question.py
26@@ -1,4 +1,4 @@
27-# Copyright 2009-2020 Canonical Ltd. This software is licensed under the
28+# Copyright 2009-2021 Canonical Ltd. This software is licensed under the
29 # GNU Affero General Public License version 3 (see the file LICENSE).
30
31 """Question views."""
32@@ -109,6 +109,7 @@ from lp.services.webapp import (
33 Link,
34 Navigation,
35 NavigationMenu,
36+ stepthrough,
37 )
38 from lp.services.webapp.authorization import check_permission
39 from lp.services.webapp.breadcrumb import Breadcrumb
40@@ -257,6 +258,23 @@ class QuestionSetNavigation(Navigation):
41 canonical_url(question, self.request), status=301)
42
43
44+class QuestionNavigation(Navigation):
45+ """Navigation for the IQuestion."""
46+
47+ usedfor = IQuestion
48+
49+ @stepthrough('messages')
50+ def traverse_messages(self, index):
51+ try:
52+ index = int(index) - 1
53+ except ValueError:
54+ return None
55+ try:
56+ return self.context.messages[index]
57+ except IndexError:
58+ return None
59+
60+
61 class QuestionBreadcrumb(Breadcrumb):
62 """Builds a breadcrumb for an `IQuestion`."""
63
64diff --git a/lib/lp/answers/configure.zcml b/lib/lp/answers/configure.zcml
65index 9735be8..a7eefcd 100644
66--- a/lib/lp/answers/configure.zcml
67+++ b/lib/lp/answers/configure.zcml
68@@ -1,4 +1,4 @@
69-<!-- Copyright 2009-2015 Canonical Ltd. This software is licensed under the
70+<!-- Copyright 2009-2021 Canonical Ltd. This software is licensed under the
71 GNU Affero General Public License version 3 (see the file LICENSE).
72 -->
73
74@@ -142,7 +142,9 @@
75
76
77 <class class=".model.questionmessage.QuestionMessage">
78- <allow interface=".interfaces.questionmessage.IQuestionMessage"/>
79+ <require permission="launchpad.Edit"
80+ interface="lp.services.messages.interfaces.message.IMessageEdit" />
81+ <allow interface=".interfaces.questionmessage.IQuestionMessageView"/>
82 </class>
83
84 <adapter
85diff --git a/lib/lp/answers/interfaces/questionmessage.py b/lib/lp/answers/interfaces/questionmessage.py
86index 176ddf8..dadd1d6 100644
87--- a/lib/lp/answers/interfaces/questionmessage.py
88+++ b/lib/lp/answers/interfaces/questionmessage.py
89@@ -1,4 +1,4 @@
90-# Copyright 2009 Canonical Ltd. This software is licensed under the
91+# Copyright 2009-2021 Canonical Ltd. This software is licensed under the
92 # GNU Affero General Public License version 3 (see the file LICENSE).
93
94 """Question message interface."""
95@@ -26,15 +26,14 @@ from lp.answers.enums import (
96 QuestionAction,
97 QuestionStatus,
98 )
99-from lp.services.messages.interfaces.message import IMessage
100-
101+from lp.services.messages.interfaces.message import (
102+ IMessage,
103+ IMessageView,
104+ )
105
106-@exported_as_webservice_entry(as_of='devel')
107-class IQuestionMessage(IMessage):
108- """A message part of a question.
109
110- It adds attributes to the IMessage interface.
111- """
112+class IQuestionMessageView(IMessageView):
113+ """Publicly visible attributes of a message part of a question."""
114 # This is really an Object field with schema=IQuestion, but that
115 # would create a circular dependency between IQuestion
116 # and IQuestionMessage
117@@ -69,3 +68,8 @@ class IQuestionMessage(IMessage):
118 description=_("Whether or not the message is visible."),
119 readonly=True),
120 as_of="devel")
121+
122+
123+@exported_as_webservice_entry(as_of='devel')
124+class IQuestionMessage(IQuestionMessageView, IMessage):
125+ """A message part of a question."""
126diff --git a/lib/lp/answers/model/questionmessage.py b/lib/lp/answers/model/questionmessage.py
127index 3acf257..3fe7c56 100644
128--- a/lib/lp/answers/model/questionmessage.py
129+++ b/lib/lp/answers/model/questionmessage.py
130@@ -1,4 +1,4 @@
131-# Copyright 2009 Canonical Ltd. This software is licensed under the
132+# Copyright 2009-2021 Canonical Ltd. This software is licensed under the
133 # GNU Affero General Public License version 3 (see the file LICENSE).
134
135 """SQLBase implementation of IQuestionMessage."""
136diff --git a/lib/lp/answers/stories/webservice.txt b/lib/lp/answers/stories/webservice.txt
137index 852e915..8ab277c 100644
138--- a/lib/lp/answers/stories/webservice.txt
139+++ b/lib/lp/answers/stories/webservice.txt
140@@ -234,6 +234,8 @@ that indicate how the message changed the question.
141 bug_attachments_collection_link: '...'
142 content: 'This is the answer'
143 date_created: '20...+00:00'
144+ date_deleted: None
145+ date_last_edited: None
146 index: 1
147 new_status: 'Answered'
148 owner_link: 'http://api.launchpad.test/devel/~contact'
149diff --git a/lib/lp/answers/tests/test_question_workflow.py b/lib/lp/answers/tests/test_question_workflow.py
150index 6450d61..66fa81b 100644
151--- a/lib/lp/answers/tests/test_question_workflow.py
152+++ b/lib/lp/answers/tests/test_question_workflow.py
153@@ -1,4 +1,4 @@
154-# Copyright 2009 Canonical Ltd. This software is licensed under the
155+# Copyright 2009-2021 Canonical Ltd. This software is licensed under the
156 # GNU Affero General Public License version 3 (see the file LICENSE).
157
158 """Test the question workflow methods.
159@@ -53,6 +53,7 @@ from lp.testing import (
160 ANONYMOUS,
161 login,
162 login_person,
163+ person_logged_in,
164 TestCase,
165 )
166 from lp.testing.fixture import ZopeEventHandlerFixture
167@@ -238,7 +239,8 @@ class BaseAnswerTrackerWorkflowTestCase(TestCase):
168 It also verifies that the question status, datelastquery (or
169 datelastresponse) were updated to reflect the time of the message.
170 """
171- self.assertTrue(verifyObject(IQuestionMessage, message))
172+ with person_logged_in(message.owner):
173+ self.assertTrue(verifyObject(IQuestionMessage, message))
174
175 self.assertEqual("Re: Help!", message.subject)
176 self.assertEqual(expected_owner, message.owner)
177diff --git a/lib/lp/bugs/browser/bugcomment.py b/lib/lp/bugs/browser/bugcomment.py
178index 2b61552..e40967f 100644
179--- a/lib/lp/bugs/browser/bugcomment.py
180+++ b/lib/lp/bugs/browser/bugcomment.py
181@@ -1,4 +1,4 @@
182-# Copyright 2006-2020 Canonical Ltd. This software is licensed under the
183+# Copyright 2006-2021 Canonical Ltd. This software is licensed under the
184 # GNU Affero General Public License version 3 (see the file LICENSE).
185
186 """Bug comment browser view classes."""
187@@ -72,7 +72,10 @@ def build_comments_from_chunks(
188 cache = get_property_cache(message)
189 if getattr(cache, 'chunks', None) is None:
190 cache.chunks = []
191- cache.chunks.append(removeSecurityProxy(chunk))
192+ # Soft-deleted messages will have None chunk here. Skip cache
193+ # filling in this case.
194+ if chunk is not None:
195+ cache.chunks.append(removeSecurityProxy(chunk))
196 bug_comment = comments.get(message.id)
197 if bug_comment is None:
198 if bugmessage.index == 0 and hide_first:
199diff --git a/lib/lp/bugs/configure.zcml b/lib/lp/bugs/configure.zcml
200index 73b99e8..b2a54ac 100644
201--- a/lib/lp/bugs/configure.zcml
202+++ b/lib/lp/bugs/configure.zcml
203@@ -1,4 +1,4 @@
204-<!-- Copyright 2009-2020 Canonical Ltd. This software is licensed under the
205+<!-- Copyright 2009-2021 Canonical Ltd. This software is licensed under the
206 GNU Affero General Public License version 3 (see the file LICENSE).
207 -->
208
209@@ -773,7 +773,9 @@
210 <class
211 class="lp.bugs.model.bugmessage.BugMessage">
212 <allow
213- interface="lp.bugs.interfaces.bugmessage.IBugMessage"/>
214+ interface="lp.bugs.interfaces.bugmessage.IBugMessageView"/>
215+ <require permission="launchpad.Edit"
216+ interface="lp.services.messages.interfaces.message.IMessageEdit" />
217 <require
218 permission="launchpad.InternalScriptsOnly"
219 set_attributes="remote_comment_id bugwatch index"/>
220diff --git a/lib/lp/bugs/interfaces/bugmessage.py b/lib/lp/bugs/interfaces/bugmessage.py
221index 98c18f3..15b3373 100644
222--- a/lib/lp/bugs/interfaces/bugmessage.py
223+++ b/lib/lp/bugs/interfaces/bugmessage.py
224@@ -1,4 +1,4 @@
225-# Copyright 2004-2020 Canonical Ltd. This software is licensed under the
226+# Copyright 2004-2021 Canonical Ltd. This software is licensed under the
227 # GNU Affero General Public License version 3 (see the file LICENSE).
228
229 """Bug message interfaces."""
230@@ -31,11 +31,14 @@ from lp.bugs.interfaces.hasbug import IHasBug
231 from lp.registry.interfaces.person import IPerson
232 from lp.services.comments.interfaces.conversation import IComment
233 from lp.services.fields import Title
234-from lp.services.messages.interfaces.message import IMessage
235+from lp.services.messages.interfaces.message import (
236+ IMessage,
237+ IMessageView,
238+ )
239
240
241-class IBugMessage(IHasBug):
242- """A link between a bug and a message."""
243+class IBugMessageView(IMessageView, IHasBug):
244+ """Public attributes for a link between a bug and a message."""
245
246 bug = Object(schema=IBug, title=u"The bug.")
247 # The index field is being populated in the DB; once complete it will be
248@@ -57,6 +60,10 @@ class IBugMessage(IHasBug):
249 title=u"The Message owner mirrored from the message.", readonly=True)
250
251
252+class IBugMessage(IBugMessageView, IMessage):
253+ """A link between a bug and a message."""
254+
255+
256 class IBugMessageSet(Interface):
257 """The set of all IBugMessages."""
258
259diff --git a/lib/lp/bugs/model/bug.py b/lib/lp/bugs/model/bug.py
260index 8f1480c..c1cc4c8 100644
261--- a/lib/lp/bugs/model/bug.py
262+++ b/lib/lp/bugs/model/bug.py
263@@ -1577,8 +1577,12 @@ class Bug(SQLBase, InformationTypeMixin):
264 # We expect:
265 # 1 bugmessage -> 1 message -> small N chunks. For now, using a wide
266 # query seems fine as we have to join out from bugmessage anyway.
267- result = Store.of(self).find((BugMessage, Message, MessageChunk),
268- Message.id == MessageChunk.messageID,
269+ # Since "soft-deleted" messages will have 0 chunks, we should use
270+ # left join here.
271+ message_join = LeftJoin(
272+ Message, MessageChunk, Message.id == MessageChunk.messageID)
273+ query = Store.of(self).using(BugMessage, message_join)
274+ result = query.find((BugMessage, Message, MessageChunk),
275 BugMessage.message_id == Message.id,
276 BugMessage.bug == self.id, *ranges)
277 result.order_by(BugMessage.index, MessageChunk.sequence)
278diff --git a/lib/lp/bugs/model/bugmessage.py b/lib/lp/bugs/model/bugmessage.py
279index 97c3015..b8e7291 100644
280--- a/lib/lp/bugs/model/bugmessage.py
281+++ b/lib/lp/bugs/model/bugmessage.py
282@@ -1,4 +1,4 @@
283-# Copyright 2009-2020 Canonical Ltd. This software is licensed under the
284+# Copyright 2009-2021 Canonical Ltd. This software is licensed under the
285 # GNU Affero General Public License version 3 (see the file LICENSE).
286
287 __metaclass__ = type
288@@ -6,6 +6,7 @@ __all__ = ['BugMessage', 'BugMessageSet']
289
290 from email.utils import make_msgid
291
292+from lazr.delegates import delegate_to
293 import six
294 from storm.properties import (
295 Int,
296@@ -22,6 +23,7 @@ from lp.bugs.interfaces.bugmessage import (
297 from lp.registry.interfaces.person import validate_public_person
298 from lp.services.database.interfaces import IStore
299 from lp.services.database.stormbase import StormBase
300+from lp.services.messages.interfaces.message import IMessage
301 from lp.services.messages.model.message import (
302 Message,
303 MessageChunk,
304@@ -29,6 +31,7 @@ from lp.services.messages.model.message import (
305
306
307 @implementer(IBugMessage)
308+@delegate_to(IMessage, context='message')
309 class BugMessage(StormBase):
310 """A table linking bugs and messages."""
311
312diff --git a/lib/lp/bugs/stories/webservice/xx-bug.txt b/lib/lp/bugs/stories/webservice/xx-bug.txt
313index 6121737..77ad62d 100644
314--- a/lib/lp/bugs/stories/webservice/xx-bug.txt
315+++ b/lib/lp/bugs/stories/webservice/xx-bug.txt
316@@ -234,6 +234,8 @@ Each bug has a collection of messages.
317 'http://.../firefox/+bug/5/comments/0/bug_attachments'
318 content: 'All ways of downloading firefox should provide...'
319 date_created: '2005-01-14T17:27:03.702622+00:00'
320+ date_deleted: None
321+ date_last_edited: None
322 owner_link: 'http://.../~name12'
323 parent_link: None
324 resource_type_link: 'http://.../#message'
325@@ -1347,6 +1349,8 @@ attachment. This is where our comment is recorded.
326 bug_attachments_collection_link: 'http://.../firefox/+bug/1/comments/2/bug_attachments'
327 content: 'The numbers you asked for.'
328 date_created: '...'
329+ date_deleted: None
330+ date_last_edited: None
331 owner_link: 'http://.../~salgado'
332 parent_link: None
333 resource_type_link: 'http://.../#message'
334diff --git a/lib/lp/code/browser/codereviewcomment.py b/lib/lp/code/browser/codereviewcomment.py
335index 24af2f8..0d0496d 100644
336--- a/lib/lp/code/browser/codereviewcomment.py
337+++ b/lib/lp/code/browser/codereviewcomment.py
338@@ -1,4 +1,4 @@
339-# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
340+# Copyright 2009-2021 Canonical Ltd. This software is licensed under the
341 # GNU Affero General Public License version 3 (see the file LICENSE).
342
343 __metaclass__ = type
344@@ -23,7 +23,10 @@ from zope.interface import (
345 implementer,
346 Interface,
347 )
348-from zope.schema import Text
349+from zope.schema import (
350+ Object,
351+ Text,
352+ )
353
354 from lp import _
355 from lp.app.browser.launchpadform import (
356@@ -40,6 +43,10 @@ from lp.services.comments.browser.messagecomment import MessageComment
357 from lp.services.comments.interfaces.conversation import IComment
358 from lp.services.config import config
359 from lp.services.librarian.interfaces import ILibraryFileAlias
360+from lp.services.messages.interfaces.message import (
361+ IMessage,
362+ IMessageEdit,
363+ )
364 from lp.services.propertycache import (
365 cachedproperty,
366 get_property_cache,
367@@ -55,10 +62,12 @@ from lp.services.webapp.interfaces import ILaunchBag
368
369 class ICodeReviewDisplayComment(IComment, ICodeReviewComment):
370 """Marker interface for displaying code review comments."""
371+ message = Object(schema=IMessage, title=_('The message.'))
372
373
374 @implementer(ICodeReviewDisplayComment)
375 @delegate_to(ICodeReviewComment, context='comment')
376+@delegate_to(IMessageEdit, context='message')
377 class CodeReviewDisplayComment(MessageComment):
378 """A code review comment or activity or both.
379
380diff --git a/lib/lp/code/browser/tests/test_codereviewcomment.py b/lib/lp/code/browser/tests/test_codereviewcomment.py
381index 799312b..e74e310 100644
382--- a/lib/lp/code/browser/tests/test_codereviewcomment.py
383+++ b/lib/lp/code/browser/tests/test_codereviewcomment.py
384@@ -1,4 +1,4 @@
385-# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
386+# Copyright 2009-2021 Canonical Ltd. This software is licensed under the
387 # GNU Affero General Public License version 3 (see the file LICENSE).
388
389 """Unit tests for CodeReviewComments."""
390@@ -56,7 +56,8 @@ class TestCodeReviewComments(TestCaseWithFactory):
391
392 display_comment = CodeReviewDisplayComment(comment)
393
394- verifyObject(ICodeReviewDisplayComment, display_comment)
395+ with person_logged_in(comment.owner):
396+ verifyObject(ICodeReviewDisplayComment, display_comment)
397
398 def test_extra_css_classes_visibility(self):
399 author = self.factory.makePerson()
400diff --git a/lib/lp/code/configure.zcml b/lib/lp/code/configure.zcml
401index 61c18c1..a373286 100644
402--- a/lib/lp/code/configure.zcml
403+++ b/lib/lp/code/configure.zcml
404@@ -1,4 +1,4 @@
405-<!-- Copyright 2009-2020 Canonical Ltd. This software is licensed under the
406+<!-- Copyright 2009-2021 Canonical Ltd. This software is licensed under the
407 GNU Affero General Public License version 3 (see the file LICENSE).
408 -->
409
410@@ -548,7 +548,9 @@
411 <!-- CodeReviewComment -->
412
413 <class class="lp.code.model.codereviewcomment.CodeReviewComment">
414- <allow interface="lp.code.interfaces.codereviewcomment.ICodeReviewComment"/>
415+ <require permission="launchpad.Edit"
416+ interface="lp.services.messages.interfaces.message.IMessageEdit" />
417+ <allow interface="lp.code.interfaces.codereviewcomment.ICodeReviewCommentView"/>
418 <allow interface="lp.code.interfaces.branchtarget.IHasBranchTarget"/>
419 </class>
420 <subscriber
421diff --git a/lib/lp/code/interfaces/codereviewcomment.py b/lib/lp/code/interfaces/codereviewcomment.py
422index f17e9a9..d32fcdd 100644
423--- a/lib/lp/code/interfaces/codereviewcomment.py
424+++ b/lib/lp/code/interfaces/codereviewcomment.py
425@@ -1,4 +1,4 @@
426-# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
427+# Copyright 2009-2021 Canonical Ltd. This software is licensed under the
428 # GNU Affero General Public License version 3 (see the file LICENSE).
429
430 """CodeReviewComment interfaces."""
431@@ -28,12 +28,15 @@ from lp import _
432 from lp.code.enums import CodeReviewVote
433 from lp.code.interfaces.branchmergeproposal import IBranchMergeProposal
434 from lp.registry.interfaces.person import IPerson
435-from lp.services.messages.interfaces.message import IMessage
436+from lp.services.messages.interfaces.message import (
437+ IMessage,
438+ IMessageCommon,
439+ IMessageEdit,
440+ )
441
442
443-@exported_as_webservice_entry()
444-class ICodeReviewComment(Interface):
445- """A link between a merge proposal and a message."""
446+class ICodeReviewCommentView(IMessageCommon):
447+ """Globally visible attributes of ICodeReviewComment."""
448
449 id = exported(
450 Int(
451@@ -69,7 +72,7 @@ class ICodeReviewComment(Interface):
452
453 message_body = exported(
454 TextLine(
455- title=_('The body of the code review message.'),
456+ title=_('Deprecated. Use "content" attribute instead.'),
457 readonly=True))
458
459 def getAttachments():
460@@ -99,6 +102,11 @@ class ICodeReviewComment(Interface):
461 """
462
463
464+@exported_as_webservice_entry()
465+class ICodeReviewComment(ICodeReviewCommentView, IMessageEdit):
466+ """A link between a merge proposal and a message."""
467+
468+
469 class ICodeReviewCommentDeletion(Interface):
470 """This interface provides deletion of CodeReviewComments.
471
472diff --git a/lib/lp/code/model/codereviewcomment.py b/lib/lp/code/model/codereviewcomment.py
473index 68b9d78..53e8a84 100644
474--- a/lib/lp/code/model/codereviewcomment.py
475+++ b/lib/lp/code/model/codereviewcomment.py
476@@ -1,4 +1,4 @@
477-# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
478+# Copyright 2009-2021 Canonical Ltd. This software is licensed under the
479 # GNU Affero General Public License version 3 (see the file LICENSE).
480
481 """The database implementation class for CodeReviewComment."""
482@@ -10,6 +10,7 @@ __all__ = [
483
484 from textwrap import TextWrapper
485
486+from lazr.delegates import delegate_to
487 from storm.locals import (
488 Int,
489 Reference,
490@@ -27,6 +28,10 @@ from lp.code.interfaces.codereviewcomment import (
491 from lp.services.database.enumcol import DBEnum
492 from lp.services.database.stormbase import StormBase
493 from lp.services.mail.signedmessage import signed_message_from_bytes
494+from lp.services.messages.interfaces.message import (
495+ IMessageCommon,
496+ IMessageEdit,
497+ )
498
499
500 def quote_text_as_email(text, width=80):
501@@ -61,7 +66,9 @@ def quote_text_as_email(text, width=80):
502 return '\n'.join(result)
503
504
505-@implementer(ICodeReviewComment, ICodeReviewCommentDeletion, IHasBranchTarget)
506+@implementer(ICodeReviewComment, ICodeReviewCommentDeletion, IHasBranchTarget,
507+ IMessageCommon, IMessageEdit)
508+@delegate_to(IMessageCommon, IMessageEdit, context='message')
509 class CodeReviewComment(StormBase):
510 """A table linking branch merge proposals and messages."""
511
512diff --git a/lib/lp/code/stories/webservice/xx-branchmergeproposal.txt b/lib/lp/code/stories/webservice/xx-branchmergeproposal.txt
513index 32f0373..58b86e7 100644
514--- a/lib/lp/code/stories/webservice/xx-branchmergeproposal.txt
515+++ b/lib/lp/code/stories/webservice/xx-branchmergeproposal.txt
516@@ -198,9 +198,13 @@ The comments on a branch merge proposal are exposed through the API.
517 as_quoted_email: '> This is great work'
518 author_link: 'http://api.launchpad.test/devel/~...'
519 branch_merge_proposal_link: 'http://.../~source/fooix/fix-it/+merge/...'
520+ content: 'This is great work'
521 date_created: '...'
522+ date_deleted: None
523+ date_last_edited: None
524 id: ...
525 message_body: 'This is great work'
526+ owner_link: 'http://...'
527 resource_type_link: 'http://.../#code_review_comment'
528 self_link: 'http://.../~source/fooix/fix-it/+merge/.../comments/...'
529 title: 'Comment on proposed merge of lp://dev/~source/fooix/fix-it into lp://dev/~target/fooix/trunk'
530@@ -216,9 +220,13 @@ The comments on a branch merge proposal are exposed through the API.
531 as_quoted_email: '> This is mediocre work.'
532 author_link: 'http://api.launchpad.test/devel/~...'
533 branch_merge_proposal_link: 'http://.../~source/fooix/fix-it/+merge/...'
534+ content: 'This is mediocre work.'
535 date_created: '...'
536+ date_deleted: None
537+ date_last_edited: None
538 id: ...
539 message_body: 'This is mediocre work.'
540+ owner_link: 'http://...'
541 resource_type_link: 'http://.../#code_review_comment'
542 self_link: 'http://.../~source/fooix/fix-it/+merge/.../comments/...'
543 title: ...
544@@ -290,9 +298,13 @@ Now the code review should be made.
545 as_quoted_email: '> This is great work'
546 author_link: 'http://api.launchpad.test/devel/~...'
547 branch_merge_proposal_link: 'http://.../~source/fooix/fix-it/+merge/...'
548+ content: 'This is great work'
549 date_created: '...'
550+ date_deleted: None
551+ date_last_edited: None
552 id: ...
553 message_body: 'This is great work'
554+ owner_link: 'http://...'
555 resource_type_link: 'http://.../#code_review_comment'
556 self_link: 'http://.../~source/fooix/fix-it/+merge/.../comments/...'
557 title: ...
558diff --git a/lib/lp/services/messages/browser/message.py b/lib/lp/services/messages/browser/message.py
559index 3b5a3c4..06800e6 100644
560--- a/lib/lp/services/messages/browser/message.py
561+++ b/lib/lp/services/messages/browser/message.py
562@@ -1,4 +1,4 @@
563-# Copyright 2009 Canonical Ltd. This software is licensed under the
564+# Copyright 2009-2021 Canonical Ltd. This software is licensed under the
565 # GNU Affero General Public License version 3 (see the file LICENSE).
566
567 """Message related view classes."""
568@@ -18,7 +18,7 @@ class QuestionMessageCanonicalUrlData:
569
570 def __init__(self, question, message):
571 self.inside = question
572- self.path = "messages/%d" % list(question.messages).index(message)
573+ self.path = "messages/%d" % message.display_index
574
575
576 @implementer(ICanonicalUrlData)
577@@ -45,14 +45,28 @@ class IndexedBugMessageCanonicalUrlData:
578 self.path = "comments/%d" % message.index
579
580
581+@implementer(ICanonicalUrlData)
582+class CodeReviewCommentCanonicalUrlData:
583+ """An optimized bug message canonical_url implementation.
584+ """
585+ rootsite = 'code'
586+
587+ def __init__(self, message):
588+ self.inside = message.branch_merge_proposal
589+ self.path = "comments/%d" % message.id
590+
591+
592 def message_to_canonical_url_data(message):
593 """This factory creates `ICanonicalUrlData` for Message."""
594 # Circular imports
595 from lp.answers.interfaces.questionmessage import IQuestionMessage
596+ from lp.code.interfaces.codereviewcomment import ICodeReviewComment
597 if IIndexedMessage.providedBy(message):
598 return IndexedBugMessageCanonicalUrlData(message)
599 elif IQuestionMessage.providedBy(message):
600 return QuestionMessageCanonicalUrlData(message.question, message)
601+ elif ICodeReviewComment.providedBy(message):
602+ return CodeReviewCommentCanonicalUrlData(message)
603 else:
604 if message.bugs.count() == 0:
605 # Will result in a ComponentLookupError
606diff --git a/lib/lp/services/messages/interfaces/message.py b/lib/lp/services/messages/interfaces/message.py
607index 70d50b7..6d4e186 100644
608--- a/lib/lp/services/messages/interfaces/message.py
609+++ b/lib/lp/services/messages/interfaces/message.py
610@@ -8,7 +8,10 @@ __all__ = [
611 'IIndexedMessage',
612 'IMessage',
613 'IMessageChunk',
614+ 'IMessageCommon',
615+ 'IMessageEdit',
616 'IMessageSet',
617+ 'IMessageView',
618 'IUserToUserEmail',
619 'IndexedMessage',
620 'InvalidEmailMessage',
621@@ -21,9 +24,11 @@ from lazr.delegates import delegate_to
622 from lazr.restful.declarations import (
623 accessor_for,
624 export_read_operation,
625+ export_write_operation,
626 exported,
627 exported_as_webservice_entry,
628 operation_for_version,
629+ operation_parameters,
630 )
631 from lazr.restful.fields import (
632 CollectionField,
633@@ -51,44 +56,62 @@ from lp.services.webservice.apihelpers import patch_reference_property
634
635 class IMessageEdit(Interface):
636
637+ @export_write_operation()
638+ @operation_parameters(
639+ new_content=TextLine(
640+ title=_("Message content"),
641+ description=_("The new message content string"),
642+ required=True))
643+ @operation_for_version("devel")
644 def editContent(new_content):
645 """Edit the content of this message, generating a new message
646 revision with the old content.
647 """
648
649+ @export_write_operation()
650+ @operation_for_version("devel")
651 def deleteContent():
652 """Deletes this message content."""
653
654
655-class IMessageView(Interface):
656- """Public attributes for message.
657-
658- This is like an email (RFC822) message, though it could be created through
659- the web as well.
660- """
661+class IMessageCommon(Interface):
662+ """Common public attributes for every IMessage implementation."""
663
664 id = Int(title=_('ID'), required=True, readonly=True)
665+
666+ chunks = Attribute(_('Message pieces'))
667+ text_contents = exported(
668+ Text(title=_('All the text/plain chunks joined together as a '
669+ 'unicode string.'), readonly=True),
670+ exported_as='content')
671+ owner = exported(
672+ Reference(title=_('Person'), schema=Interface,
673+ required=False, readonly=True))
674+
675+ revisions = Attribute(_('Message revision history'))
676 datecreated = exported(
677 Datetime(title=_('Date Created'), required=True, readonly=True),
678 exported_as='date_created')
679+ date_last_edited = exported(Datetime(
680
681- date_last_edited = Datetime(
682 title=_('When this message was last edited'), required=False,
683- readonly=True)
684-
685- date_deleted = Datetime(
686+ readonly=True))
687+ date_deleted = exported(Datetime(
688 title=_('When this message was deleted'), required=False,
689- readonly=True)
690+ readonly=True))
691+
692+
693+class IMessageView(IMessageCommon):
694+ """Public attributes for message.
695+
696+ This is like an email (RFC822) message, though it could be created through
697+ the web as well.
698+ """
699
700 subject = exported(
701 TextLine(title=_('Subject'), required=True, readonly=True))
702
703- # XXX flacoste 2006-09-08: This attribute is only used for the
704- # add form used by MessageAddView.
705 content = Text(title=_("Message"), required=True, readonly=True)
706- owner = exported(
707- Reference(title=_('Person'), schema=Interface,
708- required=False, readonly=True))
709
710 # Schema is really IMessage, but this cannot be declared here. It's
711 # fixed below after the IMessage definition is complete.
712@@ -104,15 +127,6 @@ class IMessageView(Interface):
713 title=_('Bug List'),
714 value_type=Reference(schema=Interface)) # Redefined in bug.py
715
716- chunks = Attribute(_('Message pieces'))
717-
718- revisions = Attribute(_('Message revision history'))
719-
720- text_contents = exported(
721- Text(title=_('All the text/plain chunks joined together as a '
722- 'unicode string.')),
723- exported_as='content')
724-
725 title = TextLine(
726 title=_('The message title, usually just the subject.'),
727 readonly=True)
728diff --git a/lib/lp/services/messages/model/message.py b/lib/lp/services/messages/model/message.py
729index be685d6..d20ca2f 100644
730--- a/lib/lp/services/messages/model/message.py
731+++ b/lib/lp/services/messages/model/message.py
732@@ -40,7 +40,6 @@ from sqlobject import (
733 from storm.locals import (
734 And,
735 DateTime,
736- Desc,
737 Int,
738 Max,
739 Reference,
740@@ -181,7 +180,7 @@ class Message(SQLBase):
741 return list(Store.of(self).find(
742 MessageRevision,
743 MessageRevision.message == self
744- ).order_by(Desc(MessageRevision.revision)))
745+ ).order_by(MessageRevision.revision))
746
747 def editContent(self, new_content):
748 """See `IMessage`."""
749@@ -235,7 +234,6 @@ class Message(SQLBase):
750 del get_property_cache(self).text_contents
751 del get_property_cache(self).chunks
752 del get_property_cache(self).revisions
753- return rev
754
755 def deleteContent(self):
756 """See `IMessage`."""
757diff --git a/lib/lp/services/messages/tests/test_message.py b/lib/lp/services/messages/tests/test_message.py
758index 9c2bbc7..070bc91 100644
759--- a/lib/lp/services/messages/tests/test_message.py
760+++ b/lib/lp/services/messages/tests/test_message.py
761@@ -13,6 +13,7 @@ from email.utils import (
762 )
763
764 import six
765+from testscenarios import WithScenarios
766 from testtools.matchers import (
767 Equals,
768 Is,
769@@ -20,8 +21,13 @@ from testtools.matchers import (
770 )
771 import transaction
772 from zope.security.interfaces import Unauthorized
773-from zope.security.proxy import ProxyFactory
774+from zope.security.proxy import (
775+ ProxyFactory,
776+ removeSecurityProxy,
777+ )
778
779+from lp.bugs.interfaces.bugmessage import IBugMessage
780+from lp.bugs.model.bugmessage import BugMessage
781 from lp.services.compat import message_as_bytes
782 from lp.services.database.interfaces import IStore
783 from lp.services.database.sqlbase import get_transaction_timestamp
784@@ -29,8 +35,12 @@ from lp.services.messages.model.message import (
785 MessageChunk,
786 MessageSet,
787 )
788+from lp.services.webapp.interfaces import OAuthPermission
789 from lp.testing import (
790+ admin_logged_in,
791+ api_url,
792 login,
793+ login_person,
794 person_logged_in,
795 TestCaseWithFactory,
796 )
797@@ -38,6 +48,7 @@ from lp.testing.layers import (
798 DatabaseFunctionalLayer,
799 LaunchpadFunctionalLayer,
800 )
801+from lp.testing.pages import webservice_for_person
802
803
804 class TestMessageSet(TestCaseWithFactory):
805@@ -187,16 +198,50 @@ class TestMessageSet(TestCaseWithFactory):
806 self.assertEqual(self.high_characters.decode('latin-1'), result)
807
808
809-class TestMessageEditing(TestCaseWithFactory):
810+class MessageTypeScenariosMixin(WithScenarios):
811+
812+ scenarios = [
813+ ("bug", {"message_type": "bug"}),
814+ ("question", {"message_type": "question"}),
815+ ("MP comment", {"message_type": "mp"})
816+ ]
817+
818+ def setUp(self):
819+ super(MessageTypeScenariosMixin, self).setUp()
820+ self.person = self.factory.makePerson()
821+ login_person(self.person)
822+
823+ def makeMessage(self, content=None, **kwargs):
824+ owner = kwargs.pop('owner', self.person)
825+ if self.message_type == "bug":
826+ msg = self.factory.makeBugComment(
827+ owner=owner, body=content, **kwargs)
828+ return ProxyFactory(IStore(BugMessage).find(
829+ BugMessage, BugMessage.message == msg).one())
830+ elif self.message_type == "question":
831+ question = self.factory.makeQuestion()
832+ return question.giveAnswer(owner, content)
833+ elif self.message_type == "mp":
834+ return self.factory.makeCodeReviewComment(
835+ sender=owner, body=content)
836+
837+
838+class TestMessageEditing(MessageTypeScenariosMixin, TestCaseWithFactory):
839 """Test editing scenarios for Message objects."""
840
841 layer = DatabaseFunctionalLayer
842
843- def makeMessage(self, owner=None, content=None):
844- if owner is None:
845- owner = self.factory.makePerson()
846- msg = self.factory.makeMessage(owner=owner, content=content)
847- return ProxyFactory(msg)
848+ def assertIsMessageHistory(
849+ self, msg_history, msg, rev, created_at, content, deleted_at=None):
850+ """Asserts that `msg_history` is a message history of
851+ `msg` with the given extra info.
852+ """
853+ self.assertThat(msg_history, MatchesStructure(
854+ content=Equals(content),
855+ revision=Equals(rev),
856+ message=Equals(removeSecurityProxy(msg).message),
857+ date_created=Equals(created_at),
858+ date_deleted=Equals(deleted_at)))
859
860 def test_non_owner_cannot_edit_message(self):
861 msg = self.makeMessage()
862@@ -211,12 +256,9 @@ class TestMessageEditing(TestCaseWithFactory):
863 msg.editContent("This is the new content")
864 self.assertEqual("This is the new content", msg.text_contents)
865 self.assertEqual(1, len(msg.revisions))
866- self.assertThat(msg.revisions[0], MatchesStructure(
867- content=Equals("initial content"),
868- revision=Equals(1),
869- message=Equals(msg),
870- date_created=Equals(msg.datecreated),
871- date_deleted=Is(None)))
872+ self.assertIsMessageHistory(
873+ msg.revisions[0], msg, rev=1,
874+ created_at=msg.datecreated, content="initial content")
875
876 def test_multiple_edits_revisions(self):
877 owner = self.factory.makePerson()
878@@ -226,47 +268,43 @@ class TestMessageEditing(TestCaseWithFactory):
879 first_edit_date = msg.date_last_edited
880 self.assertEqual("first edit", msg.text_contents)
881 self.assertEqual(1, len(msg.revisions))
882- self.assertThat(msg.revisions[0], MatchesStructure(
883- content=Equals("initial content"),
884- revision=Equals(1),
885- message=Equals(msg),
886- date_created=Equals(msg.datecreated),
887- date_deleted=Is(None)))
888+ self.assertIsMessageHistory(
889+ msg.revisions[0], msg, rev=1,
890+ content="initial content", created_at=msg.datecreated)
891
892 with person_logged_in(owner):
893 msg.editContent("final form")
894 self.assertEqual("final form", msg.text_contents)
895 self.assertEqual(2, len(msg.revisions))
896- self.assertThat(msg.revisions[0], MatchesStructure(
897- content=Equals("first edit"),
898- revision=Equals(2),
899- message=Equals(msg),
900- date_created=Equals(first_edit_date),
901- date_deleted=Is(None)))
902- self.assertThat(msg.revisions[1], MatchesStructure(
903- content=Equals("initial content"),
904- revision=Equals(1),
905- message=Equals(msg),
906- date_created=Equals(msg.datecreated),
907- date_deleted=Is(None)))
908+
909+ self.assertIsMessageHistory(
910+ msg.revisions[0], msg, rev=1,
911+ content="initial content", created_at=msg.datecreated)
912+ self.assertIsMessageHistory(
913+ msg.revisions[1], msg, rev=2,
914+ content="first edit", created_at=first_edit_date)
915
916 def test_edit_message_with_blobs(self):
917 # Messages with blobs should keep the blobs untouched when the
918 # content is edited.
919 owner = self.factory.makePerson()
920 msg = self.makeMessage(owner=owner, content="initial content")
921+ # The IMessage object (not the delegate one).
922+ raw_msg = removeSecurityProxy(msg).message
923+
924 files = [self.factory.makeLibraryFileAlias(db_only=True)
925 for _ in range(2)]
926 store = IStore(msg)
927 for seq, blob in enumerate(files):
928- store.add(MessageChunk(message=msg, sequence=seq + 2, blob=blob))
929+ store.add(MessageChunk(
930+ message=raw_msg, sequence=seq + 2, blob=blob))
931
932 with person_logged_in(owner):
933 msg.editContent("final form")
934 self.assertThat(msg.revisions[0], MatchesStructure(
935 content=Equals("initial content"),
936 revision=Equals(1),
937- message=Equals(msg),
938+ message=Equals(raw_msg),
939 date_created=Equals(msg.datecreated),
940 date_deleted=Is(None)))
941
942@@ -307,3 +345,73 @@ class TestMessageEditing(TestCaseWithFactory):
943 self.assertEqual(
944 get_transaction_timestamp(IStore(msg)), msg.date_deleted)
945 self.assertEqual(0, len(msg.revisions))
946+
947+
948+class TestMessageEditingAPI(MessageTypeScenariosMixin, TestCaseWithFactory):
949+ """Test editing scenarios for Message editing API."""
950+
951+ layer = DatabaseFunctionalLayer
952+
953+ def getWebservice(self, person):
954+ return webservice_for_person(
955+ person, permission=OAuthPermission.WRITE_PUBLIC,
956+ default_api_version="devel")
957+
958+ def getMessageAPIURL(self, msg):
959+ with admin_logged_in():
960+ if IBugMessage.providedBy(msg):
961+ # BugMessage has a special URL mapping that uses the
962+ # IMessage object itself.
963+ return api_url(msg.message)
964+ else:
965+ return api_url(msg)
966+
967+ def test_edit_message(self):
968+ msg = self.makeMessage(content="initial content")
969+ ws = self.getWebservice(self.person)
970+ url = self.getMessageAPIURL(msg)
971+ response = ws.named_post(
972+ url, 'editContent', new_content="the new content")
973+ self.assertEqual(200, response.status)
974+
975+ edited_obj = ws.get(url).jsonBody()
976+ self.assertEqual("the new content", edited_obj['content'])
977+ self.assertIsNone(edited_obj["date_deleted"])
978+ self.assertIsNotNone(edited_obj["date_last_edited"])
979+
980+ def test_edit_message_permission_denied_for_non_owner(self):
981+ msg = self.makeMessage(content="initial content")
982+ ws = self.getWebservice(self.factory.makePerson())
983+ url = self.getMessageAPIURL(msg)
984+ response = ws.named_post(
985+ url, 'editContent', new_content="the new content")
986+ self.assertEqual(401, response.status)
987+
988+ edited_obj = ws.get(url).jsonBody()
989+ self.assertEqual("initial content", edited_obj['content'])
990+ self.assertIsNone(edited_obj["date_deleted"])
991+ self.assertIsNone(edited_obj["date_last_edited"])
992+
993+ def test_delete_message(self):
994+ msg = self.makeMessage(content="initial content")
995+ ws = self.getWebservice(self.person)
996+ url = self.getMessageAPIURL(msg)
997+
998+ response = ws.named_post(url, 'deleteContent')
999+ self.assertEqual(200, response.status)
1000+
1001+ deleted_obj = ws.get(url).jsonBody()
1002+ self.assertEqual("", deleted_obj['content'])
1003+ self.assertIsNotNone(deleted_obj['date_deleted'])
1004+
1005+ def test_delete_message_permission_denied_for_non_owner(self):
1006+ msg = self.makeMessage(content="initial content")
1007+ ws = self.getWebservice(self.factory.makePerson())
1008+ url = self.getMessageAPIURL(msg)
1009+
1010+ response = ws.named_post(url, 'deleteContent')
1011+ self.assertEqual(401, response.status)
1012+
1013+ obj = ws.get(url).jsonBody()
1014+ self.assertEqual("initial content", obj['content'])
1015+ self.assertIsNone(obj['date_deleted'])