launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #01291
[Merge] lp:~michael.nelson/launchpad/649599-ajax-comment-on-dsdiff into lp:launchpad
Michael Nelson has proposed merging lp:~michael.nelson/launchpad/649599-ajax-comment-on-dsdiff into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Why does this diff result in the following when trying to build the wadl?
http://pastebin.ubuntu.com/502508/
It looks as though it thinks Interface is itself exported (instead of IDistroSeriesDifferenceComment)... what am I missing?
--
https://code.launchpad.net/~michael.nelson/launchpad/649599-ajax-comment-on-dsdiff/+merge/36961
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~michael.nelson/launchpad/649599-ajax-comment-on-dsdiff into lp:launchpad.
=== modified file 'lib/canonical/launchpad/interfaces/__init__.py'
--- lib/canonical/launchpad/interfaces/__init__.py 2010-09-20 14:37:52 +0000
+++ lib/canonical/launchpad/interfaces/__init__.py 2010-09-29 08:11:47 +0000
@@ -56,6 +56,7 @@
from lp.registry.interfaces.distributionmirror import *
from lp.registry.interfaces.distributionsourcepackage import *
from lp.registry.interfaces.distroseriesdifference import *
+from lp.registry.interfaces.distroseriesdifferencecomment import *
from lp.soyuz.interfaces.distributionsourcepackagecache import *
from lp.soyuz.interfaces.distributionsourcepackagerelease import *
from lp.registry.interfaces.series import *
=== modified file 'lib/lp/registry/browser/configure.zcml'
--- lib/lp/registry/browser/configure.zcml 2010-09-29 07:09:03 +0000
+++ lib/lp/registry/browser/configure.zcml 2010-09-29 08:11:47 +0000
@@ -180,6 +180,14 @@
<browser:defaultView
for="lp.registry.interfaces.distroseriesdifference.IDistroSeriesDifference"
name="+listing-distroseries-extra"/>
+ <browser:navigation
+ module="lp.registry.browser.distroseriesdifference"
+ classes="DistroSeriesDifferenceNavigation"/>
+ <browser:url
+ for="lp.registry.interfaces.distroseriesdifferencecomment.IDistroSeriesDifferenceComment"
+ path_expression="string:comments/${id}"
+ attribute_to_parent="distro_series_difference"
+ rootsite="mainsite"/>
<browser:menus
classes="
DistroSeriesFacets
=== modified file 'lib/lp/registry/browser/distroseriesdifference.py'
--- lib/lp/registry/browser/distroseriesdifference.py 2010-09-28 07:00:56 +0000
+++ lib/lp/registry/browser/distroseriesdifference.py 2010-09-29 08:11:47 +0000
@@ -9,6 +9,7 @@
]
from zope.app.form.browser.itemswidgets import RadioWidget
+from zope.component import getUtility
from zope.interface import (
implements,
Interface,
@@ -19,16 +20,40 @@
SimpleVocabulary,
)
-from canonical.launchpad.webapp import LaunchpadFormView
+from canonical.launchpad.webapp import (
+ LaunchpadFormView,
+ Navigation,
+ stepthrough,
+ )
from canonical.launchpad.webapp.authorization import check_permission
from canonical.launchpad.webapp.launchpadform import custom_widget
from lp.registry.enum import DistroSeriesDifferenceStatus
+from lp.registry.interfaces.distroseriesdifference import (
+ IDistroSeriesDifference,
+ )
+from lp.registry.interfaces.distroseriesdifferencecomment import (
+ IDistroSeriesDifferenceCommentSource,
+ )
from lp.services.comments.interfaces.conversation import (
IComment,
IConversation,
)
+class DistroSeriesDifferenceNavigation(Navigation):
+ usedfor = IDistroSeriesDifference
+
+ @stepthrough('comments')
+ def traverse_comment(self, id_str):
+ try:
+ id = int(id_str)
+ except ValueError:
+ return None
+
+ return getUtility(
+ IDistroSeriesDifferenceCommentSource).get(id)
+
+
class IDistroSeriesDifferenceForm(Interface):
"""An interface used in the browser only for displaying form elements."""
blacklist_options = Choice(vocabulary=SimpleVocabulary((
@@ -86,7 +111,7 @@
comment in self.context.getComments()]
@property
- def show_blacklist_options(self):
+ def show_edit_options(self):
"""Only show the options if an editor requests via JS."""
return self.request.is_ajax and check_permission(
'launchpad.Edit', self.context)
@@ -103,7 +128,6 @@
def __init__(self, comment):
"""Setup the attributes required by `IComment`."""
- self.message_body = comment.comment
- self.comment_author = comment.message.owner
- self.comment_date = comment.message.datecreated
- self.body_text = comment.comment
+ self.comment_author = comment.comment_author
+ self.comment_date = comment.comment_date
+ self.body_text = comment.body_text
=== modified file 'lib/lp/registry/browser/tests/test_distroseriesdifference_views.py'
--- lib/lp/registry/browser/tests/test_distroseriesdifference_views.py 2010-09-28 07:11:16 +0000
+++ lib/lp/registry/browser/tests/test_distroseriesdifference_views.py 2010-09-29 08:11:47 +0000
@@ -130,7 +130,7 @@
self.assertIs(None, ds_diff.source_pub)
self.assertIs(None, view.binary_summaries)
- def test_show_blacklist_options_non_ajax(self):
+ def test_show_edit_options_non_ajax(self):
# Blacklist options are not shown for non-ajax requests.
ds_diff = self.factory.makeDistroSeriesDifference()
@@ -138,9 +138,9 @@
with person_logged_in(ds_diff.owner):
view = create_initialized_view(
ds_diff, '+listing-distroseries-extra')
- self.assertFalse(view.show_blacklist_options)
+ self.assertFalse(view.show_edit_options)
- def test_show_blacklist_options_editor(self):
+ def test_show_edit_options_editor(self):
# Blacklist options are shown if requested by an editor via
# ajax.
ds_diff = self.factory.makeDistroSeriesDifference()
@@ -149,16 +149,16 @@
with person_logged_in(ds_diff.owner):
view = create_initialized_view(
ds_diff, '+listing-distroseries-extra', request=request)
- self.assertTrue(view.show_blacklist_options)
+ self.assertTrue(view.show_edit_options)
- def test_show_blacklist_options_non_editor(self):
+ def test_show_edit_options_non_editor(self):
# Even with a JS request, non-editors do not see the options.
ds_diff = self.factory.makeDistroSeriesDifference()
request = LaunchpadTestRequest(HTTP_X_REQUESTED_WITH='XMLHttpRequest')
view = create_initialized_view(
ds_diff, '+listing-distroseries-extra', request=request)
- self.assertFalse(view.show_blacklist_options)
+ self.assertFalse(view.show_edit_options)
class DistroSeriesDifferenceTemplateTestCase(TestCaseWithFactory):
@@ -266,7 +266,7 @@
self.assertEqual(
1, len(soup.findAll('div', {'class': 'blacklist-options'})))
- def test_blacklist_options_initial_values_NONE(self):
+ def test_blacklist_options_initial_values_none(self):
ds_diff = self.factory.makeDistroSeriesDifference()
view = create_initialized_view(ds_diff, '+listing-distroseries-extra')
@@ -274,7 +274,7 @@
# as the default value for the field.
self.assertEqual('NONE', view.initial_values.get('blacklist_options'))
- def test_blacklist_options_initial_values_CURRENT(self):
+ def test_blacklist_options_initial_values_current(self):
ds_diff = self.factory.makeDistroSeriesDifference(
status=DistroSeriesDifferenceStatus.BLACKLISTED_CURRENT)
view = create_initialized_view(ds_diff, '+listing-distroseries-extra')
@@ -283,7 +283,7 @@
DistroSeriesDifferenceStatus.BLACKLISTED_CURRENT,
view.initial_values.get('blacklist_options'))
- def test_blacklist_options_initial_values_ALWAYS(self):
+ def test_blacklist_options_initial_values_always(self):
ds_diff = self.factory.makeDistroSeriesDifference(
status=DistroSeriesDifferenceStatus.BLACKLISTED_ALWAYS)
view = create_initialized_view(ds_diff, '+listing-distroseries-extra')
=== modified file 'lib/lp/registry/browser/tests/test_distroseriesdifference_webservice.py'
--- lib/lp/registry/browser/tests/test_distroseriesdifference_webservice.py 2010-09-22 13:45:16 +0000
+++ lib/lp/registry/browser/tests/test_distroseriesdifference_webservice.py 2010-09-29 08:11:47 +0000
@@ -84,3 +84,25 @@
self.assertEqual(
DistroSeriesDifferenceStatus.NEEDS_ATTENTION,
ds_diff.status)
+
+
+class DSDCommentWebServiceTestCase(TestCaseWithFactory):
+
+ layer = AppServerLayer
+
+ def test_get_difference_comment(self):
+ # DistroSeriesDifferencesComments are available on the web service.
+ ds_diff = self.factory.makeDistroSeriesDifference()
+ from lp.testing import person_logged_in
+ from storm.store import Store
+ with person_logged_in(ds_diff.owner):
+ comment = ds_diff.addComment(ds_diff.owner, "Hey there")
+ Store.of(comment).flush()
+ transaction.commit()
+ dsd_comment_path = canonical_url(comment).replace(
+ 'http://launchpad.dev', '')
+
+ ws_diff = ws_object(self.factory.makeLaunchpadService(), comment)
+
+ self.assertTrue(
+ ws_diff.self_link.endswith(dsd_comment_path))
=== modified file 'lib/lp/registry/interfaces/distroseriesdifferencecomment.py'
--- lib/lp/registry/interfaces/distroseriesdifferencecomment.py 2010-08-31 15:56:29 +0000
+++ lib/lp/registry/interfaces/distroseriesdifferencecomment.py 2010-09-29 08:11:47 +0000
@@ -10,9 +10,14 @@
]
+from lazr.restful.declarations import (
+ export_as_webservice_entry,
+ exported,
+ )
from lazr.restful.fields import Reference
from zope.interface import Interface
from zope.schema import (
+ Datetime,
Int,
Text,
)
@@ -26,6 +31,7 @@
class IDistroSeriesDifferenceComment(Interface):
"""A comment for a distroseries difference record."""
+ export_as_webservice_entry()
id = Int(title=_('ID'), required=True, readonly=True)
@@ -38,9 +44,17 @@
IMessage, title=_("Message"), required=True, readonly=True,
description=_("A comment about this difference."))
- comment = Text(
+ body_text = exported(Text(
title=_("Comment text"), readonly=True, description=_(
- "The comment text for the related distro series difference."))
+ "The comment text for the related distro series difference.")))
+
+ comment_author = exported(Reference(
+ # Really IPerson.
+ Interface, title=_("The author of the comment."),
+ readonly=True))
+
+ comment_date = exported(Datetime(
+ title=_('Comment date.'), readonly=True))
class IDistroSeriesDifferenceCommentSource(Interface):
@@ -55,3 +69,6 @@
:param comment: The comment.
:return: A new `DistroSeriesDifferenceComment` object.
"""
+
+ def get(id):
+ """Return the `IDistroSeriesDifference` with the given id."""
=== modified file 'lib/lp/registry/model/distroseriesdifference.py'
--- lib/lp/registry/model/distroseriesdifference.py 2010-09-22 11:03:20 +0000
+++ lib/lp/registry/model/distroseriesdifference.py 2010-09-29 08:11:47 +0000
@@ -253,10 +253,10 @@
return updated
- def addComment(self, owner, comment):
+ def addComment(self, commenter, comment):
"""See `IDistroSeriesDifference`."""
return getUtility(IDistroSeriesDifferenceCommentSource).new(
- self, owner, comment)
+ self, commenter, comment)
def getComments(self):
"""See `IDistroSeriesDifference`."""
=== modified file 'lib/lp/registry/model/distroseriesdifferencecomment.py'
--- lib/lp/registry/model/distroseriesdifferencecomment.py 2010-08-31 15:56:29 +0000
+++ lib/lp/registry/model/distroseriesdifferencecomment.py 2010-09-29 08:11:47 +0000
@@ -22,7 +22,10 @@
)
from canonical.launchpad.database.message import Message, MessageChunk
-from canonical.launchpad.interfaces.lpstorm import IMasterStore
+from canonical.launchpad.interfaces.lpstorm import (
+ IMasterStore,
+ IStore,
+ )
from lp.registry.interfaces.distroseriesdifferencecomment import (
IDistroSeriesDifferenceComment,
IDistroSeriesDifferenceCommentSource,
@@ -46,10 +49,20 @@
message = Reference(message_id, 'Message.id')
@property
- def comment(self):
+ def comment_author(self):
+ """See `IDistroSeriesDifferenceComment`."""
+ return self.message.owner
+
+ @property
+ def body_text(self):
"""See `IDistroSeriesDifferenceComment`."""
return self.message.text_contents
+ @property
+ def comment_date(self):
+ """See `IDistroSeriesDifferenceComment`."""
+ return self.message.datecreated
+
@staticmethod
def new(distro_series_difference, owner, comment):
"""See `IDistroSeriesDifferenceCommentSource`."""
@@ -65,3 +78,11 @@
dsd_comment.message = message
return store.add(dsd_comment)
+
+ @staticmethod
+ def get(id):
+ """See `IDistroSeriesDifferenceCommentSource`."""
+ store = IStore(DistroSeriesDifferenceComment)
+ return store.find(
+ DistroSeriesDifferenceComment,
+ DistroSeriesDifferenceComment.id == id).one()
=== modified file 'lib/lp/registry/templates/distroseries-localdifferences.pt'
--- lib/lp/registry/templates/distroseries-localdifferences.pt 2010-09-23 11:17:45 +0000
+++ lib/lp/registry/templates/distroseries-localdifferences.pt 2010-09-29 08:11:47 +0000
@@ -68,11 +68,11 @@
<td>
<tal:comment tal:define="comment python:difference.getComments().first();"
tal:condition="comment">
- <span tal:replace="comment/comment/fmt:shorten/50">I'm on this.</span>
+ <span tal:replace="comment/body_text/fmt:shorten/50">I'm on this.</span>
<br /><span class="greyed-out greylink"><span
- tal:replace="comment/message/datecreated/fmt:approximatedate">2005-09-16</span>
+ tal:replace="comment/comment_date/fmt:approximatedate">2005-09-16</span>
by <a tal:replace="structure
- comment/message/owner/fmt:link">joesmith</a></span>
+ comment/comment_author/fmt:link">joesmith</a></span>
</tal:comment>
</td>
</tr>
=== modified file 'lib/lp/registry/templates/distroseriesdifference-listing-extra.pt'
--- lib/lp/registry/templates/distroseriesdifference-listing-extra.pt 2010-09-27 14:18:38 +0000
+++ lib/lp/registry/templates/distroseriesdifference-listing-extra.pt 2010-09-29 08:11:47 +0000
@@ -4,7 +4,7 @@
xmlns:i18n="http://xml.zope.org/namespaces/i18n";
i18n:domain="launchpad">
- <tal:show_options condition="view/show_blacklist_options">
+ <tal:show_options condition="view/show_edit_options">
<div class="blacklist-options" style="float:right">
<dl>
<dt>Blacklisted:</dt>
=== modified file 'lib/lp/registry/tests/test_distroseriesdifferencecomment.py'
--- lib/lp/registry/tests/test_distroseriesdifferencecomment.py 2010-08-31 15:56:29 +0000
+++ lib/lp/registry/tests/test_distroseriesdifferencecomment.py 2010-09-29 08:11:47 +0000
@@ -29,12 +29,12 @@
verifyObject(IDistroSeriesDifferenceComment, dsd_comment)
- def test_comment(self):
+ def test_body_text(self):
# The comment attribute returns the text of the comment.
dsd_comment = self.factory.makeDistroSeriesDifferenceComment(
comment="Wait until version 2.3")
- self.assertEqual("Wait until version 2.3", dsd_comment.comment)
+ self.assertEqual("Wait until version 2.3", dsd_comment.body_text)
def test_subject(self):
# The subject of the message is set from the distro series diff
@@ -44,3 +44,25 @@
self.assertEqual(
dsd_comment.distro_series_difference.title,
dsd_comment.message.subject)
+
+ def test_comment_author(self):
+ # The comment author just proxies the author from the message.
+ dsd_comment = self.factory.makeDistroSeriesDifferenceComment()
+
+ self.assertEqual(dsd_comment.message.owner, dsd_comment.comment_author)
+
+ def test_comment_date(self):
+ # The comment date attribute just proxies from the message.
+ dsd_comment = self.factory.makeDistroSeriesDifferenceComment()
+
+ self.assertEqual(
+ dsd_comment.message.datecreated, dsd_comment.comment_date)
+
+ def test_get_comment(self):
+ # The utility can get comments by id.
+ dsd_comment = self.factory.makeDistroSeriesDifferenceComment()
+ Store.of(dsd_comment).flush()
+
+ comment_src = getUtility(IDistroSeriesDifferenceCommentSource)
+ self.assertEqual(dsd_comment, comment_src.get(dsd_comment.id))
+
Follow ups
