Jeroen T. Vermeulen has proposed merging lp:~jtv/launchpad/bug-797151 into
lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #797151 in Launchpad itself: "Display DSD copy errors differently from
comments"
https://bugs.launchpad.net/launchpad/+bug/797151
For more details, see:
https://code.launchpad.net/~jtv/launchpad/bug-797151/+merge/64537
= Summary =
The DistroSeriesDifference pages (mainly +localpackagediffs) let archive admins
sync packages. When a sync fails, the error is stored as a
DistroSeriesDifferenceComment made by the Launchpad Janitor.
It would be nice if the latest comment as displayed on the DSD page were more
easily recognizable as an error.
== Proposed fix ==
Give the latest DSDComment its own view that knows whether the comment is an
error from Launchpad. Have the TAL render an "error" icon if the view says so.
== Pre-implementation notes ==
This is a low-priority task, but one that Julian and I both wanted behind us.
I've gotten a lot done this week and so have earned it; it also gives some
other branches a chance to go through review & land, which will make my
follow-up branch easier tomorrow.
== Implementation details ==
It would have been nice to do the same for other DSDComments (you can "fold
out" the comments list for any DSD) but they are rendered in an entirely
different way, basically as generic Messages. It might be nice to have a "this
is an automated message from Launchpad" or "this is an error" facility there,
but that's beyond the scope of this branch.
== Tests ==
{{{
./bin/test -vvc lp.registry.browser.tests.test_distroseriesdifferencecomment
}}}
== Demo and Q/A ==
Syncing packages on dogfood usually fails at the moment. Try that. The error
messages should still show up as comments, but now with little error icons.
Conversely, entering a normal comment should also work but the comment will
show up without the icon.
= Launchpad lint =
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/registry/browser/tests/test_distroseriesdifferencecomment.py
lib/lp/registry/browser/configure.zcml
lib/lp/registry/browser/distroseriesdifferencecomment.py
lib/lp/registry/templates/distroseriesdifferencecomment-latest-comment-fragment.pt
--
https://code.launchpad.net/~jtv/launchpad/bug-797151/+merge/64537
Your team Launchpad code reviewers is requested to review the proposed merge of
lp:~jtv/launchpad/bug-797151 into lp:launchpad.
=== modified file 'lib/lp/registry/browser/configure.zcml'
--- lib/lp/registry/browser/configure.zcml 2011-06-09 20:43:28 +0000
+++ lib/lp/registry/browser/configure.zcml 2011-06-14 12:37:21 +0000
@@ -200,7 +200,7 @@
<browser:page
for="..interfaces.distroseriesdifferencecomment.IDistroSeriesDifferenceComment"
template="../templates/distroseriesdifferencecomment-latest-comment-fragment.pt"
- class="canonical.launchpad.webapp.LaunchpadView"
+ class="..browser.distroseriesdifferencecomment.DistroSeriesDifferenceCommentView"
permission="zope.Public"
name="+latest-comment-fragment" />
<adapter
=== added file 'lib/lp/registry/browser/distroseriesdifferencecomment.py'
--- lib/lp/registry/browser/distroseriesdifferencecomment.py 1970-01-01 00:00:00 +0000
+++ lib/lp/registry/browser/distroseriesdifferencecomment.py 2011-06-14 12:37:21 +0000
@@ -0,0 +1,29 @@
+# Copyright 2011 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""View and helper for `DistroSeriesDifferenceComment`."""
+
+__metaclass__ = type
+
+from zope.component import getUtility
+
+from canonical.launchpad.webapp import LaunchpadView
+from lp.app.interfaces.launchpad import ILaunchpadCelebrities
+
+
+class DistroSeriesDifferenceCommentView(LaunchpadView):
+ """View class for `DistroSeriesDifferenceComment`.
+
+ :ivar css_class: The CSS class for this comment. Package copy failures
+ are stored as `DistroSeriesDifferenceComments`, but rendered to be
+ visually recognizable as errors.
+ """
+
+ def __init__(self, *args, **kwargs):
+ super(DistroSeriesDifferenceCommentView, self).__init__(
+ *args, **kwargs)
+ error_persona = getUtility(ILaunchpadCelebrities).janitor
+ if self.context.comment_author == error_persona:
+ self.css_class = "sprite error-icon"
+ else:
+ self.css_class = ""
=== modified file 'lib/lp/registry/browser/tests/test_distroseriesdifferencecomment.py'
--- lib/lp/registry/browser/tests/test_distroseriesdifferencecomment.py 2011-06-09 20:43:28 +0000
+++ lib/lp/registry/browser/tests/test_distroseriesdifferencecomment.py 2011-06-14 12:37:21 +0000
@@ -6,8 +6,10 @@
__metaclass__ = type
from lxml import html
+from zope.component import getUtility
from canonical.testing.layers import LaunchpadFunctionalLayer
+from lp.app.interfaces.launchpad import ILaunchpadCelebrities
from lp.testing import TestCaseWithFactory
from lp.testing.views import create_initialized_view
@@ -28,3 +30,22 @@
self.assertEqual(
"/~%s" % comment.comment_author.name,
root.find("span").find("a").get("href"))
+
+ def test_comment_is_rendered_with_view_css_class(self):
+ comment = self.factory.makeDistroSeriesDifferenceComment()
+ view = create_initialized_view(comment, '+latest-comment-fragment')
+ view.css_class = self.factory.getUniqueString()
+ root = html.fromstring(view())
+ self.assertEqual(view.css_class, root.find("span").get("class"))
+
+ def test_view_css_class_is_empty_by_default(self):
+ comment = self.factory.makeDistroSeriesDifferenceComment(
+ comment=self.factory.getUniqueString())
+ view = create_initialized_view(comment, '+latest-comment-fragment')
+ self.assertEqual("", view.css_class)
+
+ def test_view_css_class_has_error_sprite_if_from_janitor(self):
+ comment = self.factory.makeDistroSeriesDifferenceComment(
+ owner=getUtility(ILaunchpadCelebrities).janitor)
+ view = create_initialized_view(comment, '+latest-comment-fragment')
+ self.assertEqual("sprite error-icon", view.css_class)
=== modified file 'lib/lp/registry/templates/distroseriesdifferencecomment-latest-comment-fragment.pt'
--- lib/lp/registry/templates/distroseriesdifferencecomment-latest-comment-fragment.pt 2011-06-09 20:38:23 +0000
+++ lib/lp/registry/templates/distroseriesdifferencecomment-latest-comment-fragment.pt 2011-06-14 12:37:21 +0000
@@ -3,8 +3,15 @@
xmlns:tal="http://xml.zope.org/namespaces/tal";
xmlns:i18n="http://xml.zope.org/namespaces/i18n";
i18n:domain="launchpad">
- <span tal:replace="context/body_text/fmt:shorten/50">I'm on this.</span>
- <br /><span class="greyed-out greylink"><span
- tal:replace="context/comment_date/fmt:approximatedate">2005-09-16</span>
- by <a tal:replace="structure context/comment_author/fmt:link" /></span>
+ <span
+ tal:condition="view/css_class"
+ tal:attributes="class view/css_class"></span>
+ <tal:text tal:replace="context/body_text/fmt:shorten/50">
+ I'm on this.
+ </tal:text>
+ <br />
+ <span class="greyed-out greylink"><tal:date
+ replace="context/comment_date/fmt:approximatedate">2005-09-16</tal:date>
+ by
+ <tal:author replace="structure context/comment_author/fmt:link" /></span>
</span>
_______________________________________________
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : [email protected]
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help : https://help.launchpad.net/ListHelp
