Launchpad itself

Merge lp:~michael.nelson/launchpad/649599-ajax-comment-on-dsdiff into lp:launchpad/db-devel

  1. 649599-ajax-comment-on-dsdiff
  2. Merge into db-devel
Proposed by Michael Nelson
Status: Merged
Approved by: Jelmer Vernooij
Approved revision: no longer in the source branch.
Merged at revision: 9849
Proposed branch: lp:~michael.nelson/launchpad/649599-ajax-comment-on-dsdiff
Merge into: lp:launchpad/db-devel
Diff against target: 462 lines (+152/-28)
13 files modified
lib/canonical/launchpad/interfaces/__init__.py (+1/-0)
lib/canonical/launchpad/interfaces/_schema_circular_imports.py (+6/-0)
lib/lp/registry/browser/configure.zcml (+8/-0)
lib/lp/registry/browser/distroseriesdifference.py (+31/-6)
lib/lp/registry/browser/tests/test_distroseriesdifference_views.py (+9/-9)
lib/lp/registry/browser/tests/test_distroseriesdifference_webservice.py (+13/-0)
lib/lp/registry/interfaces/distroseriesdifference.py (+8/-1)
lib/lp/registry/interfaces/distroseriesdifferencecomment.py (+19/-2)
lib/lp/registry/model/distroseriesdifference.py (+2/-2)
lib/lp/registry/model/distroseriesdifferencecomment.py (+25/-2)
lib/lp/registry/templates/distroseries-localdifferences.pt (+3/-3)
lib/lp/registry/templates/distroseriesdifference-listing-extra.pt (+1/-1)
lib/lp/registry/tests/test_distroseriesdifferencecomment.py (+26/-2)
To merge this branch: bzr merge lp:~michael.nelson/launchpad/649599-ajax-comment-on-dsdiff
Reviewer Review Type Date Requested Status
Jelmer Vernooij (community) code Approve
Review via email: mp+36966@code.launchpad.net

Commit message

Exposes IDistroSeriesDifferenceComments and IDSD.addComment() via API

Description of the change

= Summary =
This branch exposes IDistroSeriesDifferenceComment objects for bug 649599,
getting ready to add comments via JS.

It follows on from:
https://code.edge.launchpad.net/~michael.nelson/launchpad/644328-blacklist-ui/+merge/36442

== Pre-implementation notes ==
The UI has been discussed as part of the LEP:
https://dev.launchpad.net/LEP/DerivativeDistributions

in particular, the mockup at:
https://dev.launchpad.net/LEP/DerivativeDistributions?action=AttachFile&do=get&target=derived-series-diffs_uiround2.png

(Add comment will be what the next branch adds).

== Implementation details ==

Nothing out of the ordinary.

== Tests ==
bin/test -vvm test_distroseriesdifferencecomment -m
test_distroseriesdifference_webservice

== Demo and Q/A ==
None

= Launchpad lint =

I'm updating the lint in the two files below now.

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/registry/tests/test_distroseriesdifferencecomment.py
  lib/lp/registry/browser/configure.zcml
  lib/lp/registry/templates/distroseries-localdifferences.pt
  lib/canonical/launchpad/interfaces/__init__.py
  lib/lp/registry/interfaces/distroseriesdifference.py
  lib/lp/registry/model/distroseriesdifferencecomment.py
  lib/lp/registry/browser/distroseriesdifference.py
  lib/lp/registry/browser/tests/test_distroseriesdifference_webservice.py
  lib/lp/registry/model/distroseriesdifference.py
  lib/lp/registry/interfaces/distroseriesdifferencecomment.py
  lib/lp/registry/templates/distroseriesdifference-listing-extra.pt
  lib/canonical/launchpad/interfaces/_schema_circular_imports.py
  lib/lp/registry/browser/tests/test_distroseriesdifference_views.py

./lib/lp/registry/tests/test_distroseriesdifferencecomment.py
      70: W391 blank line at end of file
      52: Line exceeds 78 characters.
./lib/canonical/launchpad/interfaces/__init__.py
      12: 'from canonical.launchpad.interfaces.launchpad import *' used; unable to detect undefined names
      ...
./lib/lp/registry/browser/tests/test_distroseriesdifference_webservice.py
      20: 'person_logged_in' imported but unused
      11: 'Store' imported but unused

To post a comment you must log in.
Revision history for this message
Jelmer Vernooij (jelmer) wrote :

Not much to add. Nice work!

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/canonical/launchpad/interfaces/__init__.py'
2--- lib/canonical/launchpad/interfaces/__init__.py 2010-09-20 14:37:52 +0000
3+++ lib/canonical/launchpad/interfaces/__init__.py 2010-09-29 09:56:01 +0000
4@@ -56,6 +56,7 @@
5 from lp.registry.interfaces.distributionmirror import *
6 from lp.registry.interfaces.distributionsourcepackage import *
7 from lp.registry.interfaces.distroseriesdifference import *
8+from lp.registry.interfaces.distroseriesdifferencecomment import *
9 from lp.soyuz.interfaces.distributionsourcepackagecache import *
10 from lp.soyuz.interfaces.distributionsourcepackagerelease import *
11 from lp.registry.interfaces.series import *
12
13=== modified file 'lib/canonical/launchpad/interfaces/_schema_circular_imports.py'
14--- lib/canonical/launchpad/interfaces/_schema_circular_imports.py 2010-09-17 10:50:51 +0000
15+++ lib/canonical/launchpad/interfaces/_schema_circular_imports.py 2010-09-29 09:56:01 +0000
16@@ -77,6 +77,9 @@
17 IDistributionSourcePackage,
18 )
19 from lp.registry.interfaces.distroseries import IDistroSeries
20+from lp.registry.interfaces.distroseriesdifferencecomment import (
21+ IDistroSeriesDifferenceComment,
22+ )
23 from lp.registry.interfaces.person import (
24 IPerson,
25 IPersonPublic,
26@@ -370,6 +373,9 @@
27 IDistroSeries, 'getPackageUploads', IPackageUpload)
28 patch_reference_property(IDistroSeries, 'parent_series', IDistroSeries)
29
30+# IDistroSeriesDifferenceComment
31+IDistroSeriesDifferenceComment['comment_author'].schema = IPerson
32+
33 # IDistroArchSeries
34 patch_reference_property(IDistroArchSeries, 'main_archive', IArchive)
35
36
37=== modified file 'lib/lp/registry/browser/configure.zcml'
38--- lib/lp/registry/browser/configure.zcml 2010-09-29 07:09:03 +0000
39+++ lib/lp/registry/browser/configure.zcml 2010-09-29 09:56:01 +0000
40@@ -180,6 +180,14 @@
41 <browser:defaultView
42 for="lp.registry.interfaces.distroseriesdifference.IDistroSeriesDifference"
43 name="+listing-distroseries-extra"/>
44+ <browser:navigation
45+ module="lp.registry.browser.distroseriesdifference"
46+ classes="DistroSeriesDifferenceNavigation"/>
47+ <browser:url
48+ for="lp.registry.interfaces.distroseriesdifferencecomment.IDistroSeriesDifferenceComment"
49+ path_expression="string:comments/${id}"
50+ attribute_to_parent="distro_series_difference"
51+ rootsite="mainsite"/>
52 <browser:menus
53 classes="
54 DistroSeriesFacets
55
56=== modified file 'lib/lp/registry/browser/distroseriesdifference.py'
57--- lib/lp/registry/browser/distroseriesdifference.py 2010-09-28 07:00:56 +0000
58+++ lib/lp/registry/browser/distroseriesdifference.py 2010-09-29 09:56:01 +0000
59@@ -9,6 +9,7 @@
60 ]
61
62 from zope.app.form.browser.itemswidgets import RadioWidget
63+from zope.component import getUtility
64 from zope.interface import (
65 implements,
66 Interface,
67@@ -19,16 +20,41 @@
68 SimpleVocabulary,
69 )
70
71-from canonical.launchpad.webapp import LaunchpadFormView
72+from canonical.launchpad.webapp import (
73+ LaunchpadFormView,
74+ Navigation,
75+ stepthrough,
76+ )
77 from canonical.launchpad.webapp.authorization import check_permission
78 from canonical.launchpad.webapp.launchpadform import custom_widget
79 from lp.registry.enum import DistroSeriesDifferenceStatus
80+from lp.registry.interfaces.distroseriesdifference import (
81+ IDistroSeriesDifference,
82+ )
83+from lp.registry.interfaces.distroseriesdifferencecomment import (
84+ IDistroSeriesDifferenceCommentSource,
85+ )
86 from lp.services.comments.interfaces.conversation import (
87 IComment,
88 IConversation,
89 )
90
91
92+class DistroSeriesDifferenceNavigation(Navigation):
93+ usedfor = IDistroSeriesDifference
94+
95+ @stepthrough('comments')
96+ def traverse_comment(self, id_str):
97+ try:
98+ id = int(id_str)
99+ except ValueError:
100+ return None
101+
102+ return getUtility(
103+ IDistroSeriesDifferenceCommentSource).getForDifference(
104+ self.context, id)
105+
106+
107 class IDistroSeriesDifferenceForm(Interface):
108 """An interface used in the browser only for displaying form elements."""
109 blacklist_options = Choice(vocabulary=SimpleVocabulary((
110@@ -86,7 +112,7 @@
111 comment in self.context.getComments()]
112
113 @property
114- def show_blacklist_options(self):
115+ def show_edit_options(self):
116 """Only show the options if an editor requests via JS."""
117 return self.request.is_ajax and check_permission(
118 'launchpad.Edit', self.context)
119@@ -103,7 +129,6 @@
120
121 def __init__(self, comment):
122 """Setup the attributes required by `IComment`."""
123- self.message_body = comment.comment
124- self.comment_author = comment.message.owner
125- self.comment_date = comment.message.datecreated
126- self.body_text = comment.comment
127+ self.comment_author = comment.comment_author
128+ self.comment_date = comment.comment_date
129+ self.body_text = comment.body_text
130
131=== modified file 'lib/lp/registry/browser/tests/test_distroseriesdifference_views.py'
132--- lib/lp/registry/browser/tests/test_distroseriesdifference_views.py 2010-09-28 07:11:16 +0000
133+++ lib/lp/registry/browser/tests/test_distroseriesdifference_views.py 2010-09-29 09:56:01 +0000
134@@ -130,7 +130,7 @@
135 self.assertIs(None, ds_diff.source_pub)
136 self.assertIs(None, view.binary_summaries)
137
138- def test_show_blacklist_options_non_ajax(self):
139+ def test_show_edit_options_non_ajax(self):
140 # Blacklist options are not shown for non-ajax requests.
141 ds_diff = self.factory.makeDistroSeriesDifference()
142
143@@ -138,9 +138,9 @@
144 with person_logged_in(ds_diff.owner):
145 view = create_initialized_view(
146 ds_diff, '+listing-distroseries-extra')
147- self.assertFalse(view.show_blacklist_options)
148+ self.assertFalse(view.show_edit_options)
149
150- def test_show_blacklist_options_editor(self):
151+ def test_show_edit_options_editor(self):
152 # Blacklist options are shown if requested by an editor via
153 # ajax.
154 ds_diff = self.factory.makeDistroSeriesDifference()
155@@ -149,16 +149,16 @@
156 with person_logged_in(ds_diff.owner):
157 view = create_initialized_view(
158 ds_diff, '+listing-distroseries-extra', request=request)
159- self.assertTrue(view.show_blacklist_options)
160+ self.assertTrue(view.show_edit_options)
161
162- def test_show_blacklist_options_non_editor(self):
163+ def test_show_edit_options_non_editor(self):
164 # Even with a JS request, non-editors do not see the options.
165 ds_diff = self.factory.makeDistroSeriesDifference()
166
167 request = LaunchpadTestRequest(HTTP_X_REQUESTED_WITH='XMLHttpRequest')
168 view = create_initialized_view(
169 ds_diff, '+listing-distroseries-extra', request=request)
170- self.assertFalse(view.show_blacklist_options)
171+ self.assertFalse(view.show_edit_options)
172
173
174 class DistroSeriesDifferenceTemplateTestCase(TestCaseWithFactory):
175@@ -266,7 +266,7 @@
176 self.assertEqual(
177 1, len(soup.findAll('div', {'class': 'blacklist-options'})))
178
179- def test_blacklist_options_initial_values_NONE(self):
180+ def test_blacklist_options_initial_values_none(self):
181 ds_diff = self.factory.makeDistroSeriesDifference()
182 view = create_initialized_view(ds_diff, '+listing-distroseries-extra')
183
184@@ -274,7 +274,7 @@
185 # as the default value for the field.
186 self.assertEqual('NONE', view.initial_values.get('blacklist_options'))
187
188- def test_blacklist_options_initial_values_CURRENT(self):
189+ def test_blacklist_options_initial_values_current(self):
190 ds_diff = self.factory.makeDistroSeriesDifference(
191 status=DistroSeriesDifferenceStatus.BLACKLISTED_CURRENT)
192 view = create_initialized_view(ds_diff, '+listing-distroseries-extra')
193@@ -283,7 +283,7 @@
194 DistroSeriesDifferenceStatus.BLACKLISTED_CURRENT,
195 view.initial_values.get('blacklist_options'))
196
197- def test_blacklist_options_initial_values_ALWAYS(self):
198+ def test_blacklist_options_initial_values_always(self):
199 ds_diff = self.factory.makeDistroSeriesDifference(
200 status=DistroSeriesDifferenceStatus.BLACKLISTED_ALWAYS)
201 view = create_initialized_view(ds_diff, '+listing-distroseries-extra')
202
203=== modified file 'lib/lp/registry/browser/tests/test_distroseriesdifference_webservice.py'
204--- lib/lp/registry/browser/tests/test_distroseriesdifference_webservice.py 2010-09-22 13:45:16 +0000
205+++ lib/lp/registry/browser/tests/test_distroseriesdifference_webservice.py 2010-09-29 09:56:01 +0000
206@@ -84,3 +84,16 @@
207 self.assertEqual(
208 DistroSeriesDifferenceStatus.NEEDS_ATTENTION,
209 ds_diff.status)
210+
211+ def test_addComment(self):
212+ # Comments can be added via the API
213+ ds_diff = self.factory.makeDistroSeriesDifference()
214+ ws_diff = ws_object(self.factory.makeLaunchpadService(
215+ ds_diff.owner), ds_diff)
216+
217+ result = ws_diff.addComment(comment='Hey there')
218+
219+ self.assertEqual('Hey there', result['body_text'])
220+ self.assertTrue(
221+ result['resource_type_link'].endswith(
222+ '#distro_series_difference_comment'))
223
224=== modified file 'lib/lp/registry/interfaces/distroseriesdifference.py'
225--- lib/lp/registry/interfaces/distroseriesdifference.py 2010-09-23 11:17:45 +0000
226+++ lib/lp/registry/interfaces/distroseriesdifference.py 2010-09-29 09:56:01 +0000
227@@ -14,10 +14,12 @@
228 ]
229
230 from lazr.restful.declarations import (
231+ call_with,
232 export_as_webservice_entry,
233 export_write_operation,
234 exported,
235 operation_parameters,
236+ REQUEST_USER,
237 )
238 from lazr.restful.fields import Reference
239 from zope.interface import Interface
240@@ -25,6 +27,7 @@
241 Bool,
242 Choice,
243 Int,
244+ Text,
245 TextLine,
246 )
247
248@@ -134,7 +137,11 @@
249 class IDistroSeriesDifferenceEdit(Interface):
250 """Difference attributes requiring launchpad.Edit."""
251
252- def addComment(owner, comment):
253+ @call_with(commenter=REQUEST_USER)
254+ @operation_parameters(
255+ comment=Text(title=_("Comment text"), required=True))
256+ @export_write_operation()
257+ def addComment(commenter, comment):
258 """Add a comment on this difference."""
259
260 @operation_parameters(
261
262=== modified file 'lib/lp/registry/interfaces/distroseriesdifferencecomment.py'
263--- lib/lp/registry/interfaces/distroseriesdifferencecomment.py 2010-08-31 15:56:29 +0000
264+++ lib/lp/registry/interfaces/distroseriesdifferencecomment.py 2010-09-29 09:56:01 +0000
265@@ -10,9 +10,14 @@
266 ]
267
268
269+from lazr.restful.declarations import (
270+ export_as_webservice_entry,
271+ exported,
272+ )
273 from lazr.restful.fields import Reference
274 from zope.interface import Interface
275 from zope.schema import (
276+ Datetime,
277 Int,
278 Text,
279 )
280@@ -26,6 +31,7 @@
281
282 class IDistroSeriesDifferenceComment(Interface):
283 """A comment for a distroseries difference record."""
284+ export_as_webservice_entry()
285
286 id = Int(title=_('ID'), required=True, readonly=True)
287
288@@ -38,9 +44,17 @@
289 IMessage, title=_("Message"), required=True, readonly=True,
290 description=_("A comment about this difference."))
291
292- comment = Text(
293+ body_text = exported(Text(
294 title=_("Comment text"), readonly=True, description=_(
295- "The comment text for the related distro series difference."))
296+ "The comment text for the related distro series difference.")))
297+
298+ comment_author = exported(Reference(
299+ # Really IPerson.
300+ Interface, title=_("The author of the comment."),
301+ readonly=True))
302+
303+ comment_date = exported(Datetime(
304+ title=_('Comment date.'), readonly=True))
305
306
307 class IDistroSeriesDifferenceCommentSource(Interface):
308@@ -55,3 +69,6 @@
309 :param comment: The comment.
310 :return: A new `DistroSeriesDifferenceComment` object.
311 """
312+
313+ def getForDifference(distro_series_difference, id):
314+ """Return the `IDistroSeriesDifferenceComment` with the given id."""
315
316=== modified file 'lib/lp/registry/model/distroseriesdifference.py'
317--- lib/lp/registry/model/distroseriesdifference.py 2010-09-22 11:03:20 +0000
318+++ lib/lp/registry/model/distroseriesdifference.py 2010-09-29 09:56:01 +0000
319@@ -253,10 +253,10 @@
320
321 return updated
322
323- def addComment(self, owner, comment):
324+ def addComment(self, commenter, comment):
325 """See `IDistroSeriesDifference`."""
326 return getUtility(IDistroSeriesDifferenceCommentSource).new(
327- self, owner, comment)
328+ self, commenter, comment)
329
330 def getComments(self):
331 """See `IDistroSeriesDifference`."""
332
333=== modified file 'lib/lp/registry/model/distroseriesdifferencecomment.py'
334--- lib/lp/registry/model/distroseriesdifferencecomment.py 2010-08-31 15:56:29 +0000
335+++ lib/lp/registry/model/distroseriesdifferencecomment.py 2010-09-29 09:56:01 +0000
336@@ -22,7 +22,10 @@
337 )
338
339 from canonical.launchpad.database.message import Message, MessageChunk
340-from canonical.launchpad.interfaces.lpstorm import IMasterStore
341+from canonical.launchpad.interfaces.lpstorm import (
342+ IMasterStore,
343+ IStore,
344+ )
345 from lp.registry.interfaces.distroseriesdifferencecomment import (
346 IDistroSeriesDifferenceComment,
347 IDistroSeriesDifferenceCommentSource,
348@@ -46,10 +49,20 @@
349 message = Reference(message_id, 'Message.id')
350
351 @property
352- def comment(self):
353+ def comment_author(self):
354+ """See `IDistroSeriesDifferenceComment`."""
355+ return self.message.owner
356+
357+ @property
358+ def body_text(self):
359 """See `IDistroSeriesDifferenceComment`."""
360 return self.message.text_contents
361
362+ @property
363+ def comment_date(self):
364+ """See `IDistroSeriesDifferenceComment`."""
365+ return self.message.datecreated
366+
367 @staticmethod
368 def new(distro_series_difference, owner, comment):
369 """See `IDistroSeriesDifferenceCommentSource`."""
370@@ -65,3 +78,13 @@
371 dsd_comment.message = message
372
373 return store.add(dsd_comment)
374+
375+ @staticmethod
376+ def getForDifference(distro_series_difference, id):
377+ """See `IDistroSeriesDifferenceCommentSource`."""
378+ store = IStore(DistroSeriesDifferenceComment)
379+ DSDComment = DistroSeriesDifferenceComment
380+ return store.find(
381+ DSDComment,
382+ DSDComment.distro_series_difference == distro_series_difference,
383+ DSDComment.id == id).one()
384
385=== modified file 'lib/lp/registry/templates/distroseries-localdifferences.pt'
386--- lib/lp/registry/templates/distroseries-localdifferences.pt 2010-09-23 11:17:45 +0000
387+++ lib/lp/registry/templates/distroseries-localdifferences.pt 2010-09-29 09:56:01 +0000
388@@ -68,11 +68,11 @@
389 <td>
390 <tal:comment tal:define="comment python:difference.getComments().first();"
391 tal:condition="comment">
392- <span tal:replace="comment/comment/fmt:shorten/50">I'm on this.</span>
393+ <span tal:replace="comment/body_text/fmt:shorten/50">I'm on this.</span>
394 <br /><span class="greyed-out greylink"><span
395- tal:replace="comment/message/datecreated/fmt:approximatedate">2005-09-16</span>
396+ tal:replace="comment/comment_date/fmt:approximatedate">2005-09-16</span>
397 by <a tal:replace="structure
398- comment/message/owner/fmt:link">joesmith</a></span>
399+ comment/comment_author/fmt:link">joesmith</a></span>
400 </tal:comment>
401 </td>
402 </tr>
403
404=== modified file 'lib/lp/registry/templates/distroseriesdifference-listing-extra.pt'
405--- lib/lp/registry/templates/distroseriesdifference-listing-extra.pt 2010-09-27 14:18:38 +0000
406+++ lib/lp/registry/templates/distroseriesdifference-listing-extra.pt 2010-09-29 09:56:01 +0000
407@@ -4,7 +4,7 @@
408 xmlns:i18n="http://xml.zope.org/namespaces/i18n"
409 i18n:domain="launchpad">
410
411- <tal:show_options condition="view/show_blacklist_options">
412+ <tal:show_options condition="view/show_edit_options">
413 <div class="blacklist-options" style="float:right">
414 <dl>
415 <dt>Blacklisted:</dt>
416
417=== modified file 'lib/lp/registry/tests/test_distroseriesdifferencecomment.py'
418--- lib/lp/registry/tests/test_distroseriesdifferencecomment.py 2010-08-31 15:56:29 +0000
419+++ lib/lp/registry/tests/test_distroseriesdifferencecomment.py 2010-09-29 09:56:01 +0000
420@@ -29,12 +29,12 @@
421
422 verifyObject(IDistroSeriesDifferenceComment, dsd_comment)
423
424- def test_comment(self):
425+ def test_body_text(self):
426 # The comment attribute returns the text of the comment.
427 dsd_comment = self.factory.makeDistroSeriesDifferenceComment(
428 comment="Wait until version 2.3")
429
430- self.assertEqual("Wait until version 2.3", dsd_comment.comment)
431+ self.assertEqual("Wait until version 2.3", dsd_comment.body_text)
432
433 def test_subject(self):
434 # The subject of the message is set from the distro series diff
435@@ -44,3 +44,27 @@
436 self.assertEqual(
437 dsd_comment.distro_series_difference.title,
438 dsd_comment.message.subject)
439+
440+ def test_comment_author(self):
441+ # The comment author just proxies the author from the message.
442+ dsd_comment = self.factory.makeDistroSeriesDifferenceComment()
443+
444+ self.assertEqual(
445+ dsd_comment.message.owner, dsd_comment.comment_author)
446+
447+ def test_comment_date(self):
448+ # The comment date attribute just proxies from the message.
449+ dsd_comment = self.factory.makeDistroSeriesDifferenceComment()
450+
451+ self.assertEqual(
452+ dsd_comment.message.datecreated, dsd_comment.comment_date)
453+
454+ def test_getForDifference(self):
455+ # The utility can get comments by id.
456+ dsd_comment = self.factory.makeDistroSeriesDifferenceComment()
457+ Store.of(dsd_comment).flush()
458+
459+ comment_src = getUtility(IDistroSeriesDifferenceCommentSource)
460+ self.assertEqual(
461+ dsd_comment, comment_src.getForDifference(
462+ dsd_comment.distro_series_difference, dsd_comment.id))

Subscribers

People subscribed via source and target branches

to status/vote changes: