Merge lp:~jtv/launchpad/translationimportqueueentry-info into lp:launchpad

Proposed by Jeroen T. Vermeulen
Status: Merged
Approved by: Jeroen T. Vermeulen
Approved revision: not available
Merged at revision: not available
Proposed branch: lp:~jtv/launchpad/translationimportqueueentry-info
Merge into: lp:launchpad
Diff against target: 586 lines (+459/-40)
7 files modified
lib/lp/translations/browser/configure.zcml (+1/-0)
lib/lp/translations/browser/tests/test_translationimportqueueentry.py (+164/-0)
lib/lp/translations/browser/translationimportqueue.py (+86/-4)
lib/lp/translations/stories/importqueue/xx-entry-details.txt (+106/-0)
lib/lp/translations/stories/importqueue/xx-entry-error-output.txt (+46/-0)
lib/lp/translations/templates/translationimportqueueentry-index.pt (+13/-0)
lib/lp/translations/templates/translationimportqueueentry-portlet-details.pt (+43/-36)
To merge this branch: bzr merge lp:~jtv/launchpad/translationimportqueueentry-info
Reviewer Review Type Date Requested Status
Curtis Hovey (community) ui Approve
Henning Eggers (community) code Approve
Review via email: mp+18963@code.launchpad.net

Commit message

Show translation upload details on approval form.

To post a comment you must log in.
Revision history for this message
Jeroen T. Vermeulen (jtv) wrote :

= Bug 519556 =

When reviewing translations uploads, we translations admins often want quick access to the following information on the approval page:
 * The uploader.
 * The file's contents.
 * The project: what's its license status, is it already using Translations?
 * The release series: does it have any templates yet, and which ones?
 * When was the entry last updated? The current UI fails to show this anywhere.

In addition, it may occasionally be convenient to have a copy of the entry's error_output shown. That makes this page a great place to go when analyzing failures.

In this branch I add all of this information. It's at the bottom of the page, since I want it to be unobtrusive. It wouldn't do to force admins to scroll down before they get to the input fields. (Only translations admins and the Ubuntu translation coordinates can access this page).

For Q/A, I intend to visit a whole bunch of pages for translation import queue entries and see that they all make sense.

To test:
{{{
./bin/test -vv -t importqueue/xx-entry
}}}

No lint.

Jeroen

Revision history for this message
Henning Eggers (henninge) wrote :
Download full text (15.5 KiB)

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi Jeroen,

thanks for working late and creating this branch. Great work!

Am 10.02.2010 00:36, Jeroen T. Vermeulen schrieb:
> = Bug 519556 =
>
> When reviewing translations uploads, we translations admins often want quick access to the following information on the approval page:
> * The uploader.
> * The file's contents.
> * The project: what's its license status, is it already using Translations?
> * The release series: does it have any templates yet, and which ones?
> * When was the entry last updated? The current UI fails to show this anywhere.

Wonderful! That is very helpful information!

>
> In addition, it may occasionally be convenient to have a copy of the entry's error_output shown. That makes this page a great place to go when analyzing failures.

Actually, as you know, I have a branch ready that displays the
error_output in a text field as part of the form, so this part might go
out again, soon. No offence meant. ;-)

>
> In this branch I add all of this information. It's at the bottom of the page, since I want it to be unobtrusive. It wouldn't do to force admins to scroll down before they get to the input fields. (Only translations admins and the Ubuntu translation coordinates can access this page).

Yes, I agree with that, although a two-column layout wouldn't have been
bad, either. The form would go on the left, the information on the
right. But I am not sure if that is easy to do and would fit with our UI
guidelines.

As I can see, you managed to pull this off without adding any view code.
Impressive. ;-) I am going to ask you, though, to move some things to
the view code to keep the TAL clearer. Hope you can go along with that.

 review needs-fixing code

Cheers,
Henning

> === added file 'lib/lp/translations/stories/importqueue/xx-entry-details.txt'
> --- lib/lp/translations/stories/importqueue/xx-entry-details.txt 1970-01-01 00:00:00 +0000
> +++ lib/lp/translations/stories/importqueue/xx-entry-details.txt 2010-02-09 23:36:23 +0000
> @@ -0,0 +1,121 @@
> += Entry details =
> +
> +The translation import queue entry page shows various details about an
> +entry and its target that may be helpful in queue review.
> +
> + >>> from zope.security.proxy import removeSecurityProxy
> + >>> from lp.translations.model.translationimportqueue import (
> + ... TranslationImportQueue)
> +
> + >>> filename = 'po/foo.pot'
> +
> + >>> login(ANONYMOUS)
> + >>> queue = TranslationImportQueue()
> + >>> product = factory.makeProduct()
> + >>> removeSecurityProxy(product).official_rosetta = True

I wonder if removeSecurityProxy would be necessary here if you logged in
as foo.bar?

> + >>> trunk = product.getSeries('trunk')
> + >>> uploader = factory.makePerson()
> + >>> entry = queue.addOrUpdateEntry(
> + ... filename, '# empty', False, uploader, productseries=trunk)
> + >>> entry_url = canonical_url(entry, rootsite='translations')
> + >>> logout()
> +
> + >>> admin_browser.open(entry_url)
> + >>> details = find_tag_by_id(admin_browser.contents, 'portlet-details')
> + >>> details_text = extract_text(details.renderContents())
> + ...

review: Needs Fixing (code)
Revision history for this message
Henning Eggers (henninge) wrote :

For the UI review, here are two screenshots, one of an entry for a template file with error outpout, one for a normal translation file without error output. The review is just about the stuff that comes under the actions.

http://people.canonical.com/~henninge/screenshots/import-queue-entry-info-pot.png
http://people.canonical.com/~henninge/screenshots/import-queue-entry-info-po.png

Revision history for this message
Jeroen T. Vermeulen (jtv) wrote :
Download full text (7.9 KiB)

Hi Henning, thank you for the surprise review! And a thorough job you did of it, too. Glad you like it, because this branch did contribute to a headache. :-)

> Actually, as you know, I have a branch ready that displays the
> error_output in a text field as part of the form, so this part might go
> out again, soon. No offence meant. ;-)

I considered that. But the error_output can be pages and pages of text in some cases. Viewing those in the queue UI isn't very convenient, and neither is scrolling through long and wide text in an input box. So I figured we might sometimes appreciate having it displayed more plainly. It's an extra, so I did put it at the bottom of the page.

> Yes, I agree with that, although a two-column layout wouldn't have been
> bad, either. The form would go on the left, the information on the
> right. But I am not sure if that is easy to do and would fit with our UI
> guidelines.

I'm not sure we can do that with generic forms. I did try it with the yui table layout, but had no luck.

In any case, this is nice and compact, no?

> As I can see, you managed to pull this off without adding any view code.
> Impressive. ;-) I am going to ask you, though, to move some things to
> the view code to keep the TAL clearer. Hope you can go along with that.

Quite right you are. TAL can be nice for prototyping though.

> > === added file 'lib/lp/translations/stories/importqueue/xx-entry-
> details.txt'
> > --- lib/lp/translations/stories/importqueue/xx-entry-details.txt
> 1970-01-01 00:00:00 +0000
> > +++ lib/lp/translations/stories/importqueue/xx-entry-details.txt
> 2010-02-09 23:36:23 +0000
> > @@ -0,0 +1,121 @@
> > += Entry details =
> > +
> > +The translation import queue entry page shows various details about an
> > +entry and its target that may be helpful in queue review.
> > +
> > + >>> from zope.security.proxy import removeSecurityProxy
> > + >>> from lp.translations.model.translationimportqueue import (
> > + ... TranslationImportQueue)
> > +
> > + >>> filename = 'po/foo.pot'
> > +
> > + >>> login(ANONYMOUS)
> > + >>> queue = TranslationImportQueue()
> > + >>> product = factory.makeProduct()
> > + >>> removeSecurityProxy(product).official_rosetta = True
>
> I wonder if removeSecurityProxy would be necessary here if you logged in
> as foo.bar?

Maybe not, but it's yet another dependency on the test data with implicit meaning ("logging in as this user because it's an admin"). Stripping the proxy was easy enough.

> > +In that case, the product is also shown to have translatable series.
> > +
> > + >>> print details_text
> > + Upload attached to
> > + ...
> > + Project has translatable series.
> > + ...
> > +
>
> Would it be too much to list these series here?

Almost, but I did it anyway. :-) It now lists up to three series (with linkified names), and if there's more, it adds an ellipsis.

> > +The string "1 template" neatly adjusts to the actual number of
> > +templates.
> > +
> > + >>> login(ANONYMOUS)
> > + >>> template = factory.makePOTemplate(productseries=trunk)
> > + >>> logout()
> > +
> > + >>> admin_browser.open(entry_url)
> > + >>> detail...

Read more...

Revision history for this message
Henning Eggers (henninge) wrote :
Download full text (12.2 KiB)

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi Jeroen,
thanks for this improvement. I have a few little issues but please land
this when you are done with them.

 review approve code

Cheers,
Henning

> === added file 'lib/lp/translations/browser/tests/test_translationimportqueueentry.py'
> --- lib/lp/translations/browser/tests/test_translationimportqueueentry.py 1970-01-01 00:00:00 +0000
> +++ lib/lp/translations/browser/tests/test_translationimportqueueentry.py 2010-02-10 13:06:21 +0000
> @@ -0,0 +1,165 @@
> +# Copyright 2010 Canonical Ltd. This software is licensed under the
> +# GNU Affero General Public License version 3 (see the file LICENSE).
> +
> +"""Unit tests for translation import queue views."""
> +
> +from datetime import datetime
> +from pytz import timezone
> +from unittest import TestLoader
> +
> +from zope.component import getMultiAdapter
> +from zope.security.proxy import removeSecurityProxy
> +
> +from canonical.testing import LaunchpadFunctionalLayer
> +
> +from canonical.launchpad.layers import TranslationsLayer, setFirstLayer
> +from canonical.launchpad.webapp import canonical_url
> +from canonical.launchpad.webapp.servers import LaunchpadTestRequest
> +from lp.registry.model.sourcepackage import SourcePackage
> +from lp.testing import TestCaseWithFactory
> +from lp.translations.model.translationimportqueue import (
> + TranslationImportQueue)

Don't you get an warnings about this? Should test code be importing from
 model code? I thought I remember that rule (and it makes sense if we
profess to test against Interfaces).

> +
> +
> +class TestTranslationImportQueueEntryView(TestCaseWithFactory):
> + """Tests for the queue entry review form."""
> +
> + layer = LaunchpadFunctionalLayer
> +
> + def setUp(self):
> + super(TestTranslationImportQueueEntryView, self).setUp(
> + '<email address hidden>')
> + self.queue = TranslationImportQueue()

I think this should really be:
  self.queue = getUtility(ITranslationImportQueue)

> + self.uploader = self.factory.makePerson()
> +
> + def _makeProductSeries(self):
> + """Set up a product series for a translatable product."""
> + product = self.factory.makeProduct()
> + product.official_rosetta = True
> + return product.getSeries('trunk')
> +
> + def _makeView(self, entry):
> + """Create view for a queue entry."""
> + request = LaunchpadTestRequest()
> + setFirstLayer(request, TranslationsLayer)
> + view = getMultiAdapter((entry, request), name='+index')
> + view.initialize()
> + return view
> +
> + def _makeEntry(self, productseries=None, distroseries=None,
> + sourcepackagename=None):
> + filename = self.factory.getUniqueString() + '.pot'
> + contents = self.factory.getUniqueString()
> + entry = self.queue.addOrUpdateEntry(
> + filename, contents, False, self.uploader,
> + productseries=productseries, distroseries=distroseries,
> + sourcepackagename=sourcepackagename)
> + return removeSecurityProxy(entry)
> +
> + def test_import_target_productseries(self):
> + # If the ...

review: Approve (code)
Revision history for this message
Jeroen T. Vermeulen (jtv) wrote :
Download full text (5.6 KiB)

> > === added file
> 'lib/lp/translations/browser/tests/test_translationimportqueueentry.py'
> > --- lib/lp/translations/browser/tests/test_translationimportqueueentry.py
> 1970-01-01 00:00:00 +0000
> > +++ lib/lp/translations/browser/tests/test_translationimportqueueentry.py
> 2010-02-10 13:06:21 +0000
> > @@ -0,0 +1,165 @@
> > +# Copyright 2010 Canonical Ltd. This software is licensed under the
> > +# GNU Affero General Public License version 3 (see the file LICENSE).
> > +
> > +"""Unit tests for translation import queue views."""
> > +
> > +from datetime import datetime
> > +from pytz import timezone
> > +from unittest import TestLoader
> > +
> > +from zope.component import getMultiAdapter
> > +from zope.security.proxy import removeSecurityProxy
> > +
> > +from canonical.testing import LaunchpadFunctionalLayer
> > +
> > +from canonical.launchpad.layers import TranslationsLayer, setFirstLayer
> > +from canonical.launchpad.webapp import canonical_url
> > +from canonical.launchpad.webapp.servers import LaunchpadTestRequest
> > +from lp.registry.model.sourcepackage import SourcePackage
> > +from lp.testing import TestCaseWithFactory
> > +from lp.translations.model.translationimportqueue import (
> > + TranslationImportQueue)
>
> Don't you get an warnings about this? Should test code be importing from
> model code? I thought I remember that rule (and it makes sense if we
> profess to test against Interfaces).

No warnings, but now that you mention it, I'm not sure this is allowed.
Changed.

> > +class TestTranslationImportQueueEntryView(TestCaseWithFactory):
> > + """Tests for the queue entry review form."""
> > +
> > + layer = LaunchpadFunctionalLayer
> > +
> > + def setUp(self):
> > + super(TestTranslationImportQueueEntryView, self).setUp(
> > + '<email address hidden>')
> > + self.queue = TranslationImportQueue()
>
> I think this should really be:
> self.queue = getUtility(ITranslationImportQueue)

Done.

> > + def test_import_target_sourcepackage(self):
> > + # If the entry has a DistroSeries and a SourcePackageName, the
> > + # import_target is the corresponding SourcePackage.
> > + series = self.factory.makeDistroSeries()
> > + packagename = self.factory.makeSourcePackageName()
> > + package = SourcePackage(packagename, series)
>
> package = self.factory.makeSourcePackage(packagename, series)
>
> Works just as well ... ;-)

Oh, nice! Saves me an import.

> > + # No translatable series.
> > + series_text = view.product_translatable_series
> > + self.assertEqual("Project has no translatable series.",
> series_text)
> > +
> > + # One translatable series.
> > + extra_series = self.factory.makeProductSeries(product=product)
> > + self.factory.makePOTemplate(productseries=extra_series)
> > + series_text = view.product_translatable_series
> > + self.assertIn("Project has translatable series:", series_text)
> > + # A link follows, and the sentence ends in a period.
> > + self.assertEqual('</a>.', series_text[-5:])
>
> I am not all too happy about this testing style and wonder if doing this
...

Read more...

Revision history for this message
Jeroen T. Vermeulen (jtv) wrote :

UI reviewer: the "translatable series" text now links to the individual series. Updated screenshots at http://people.canonical.com/~jtv/translationimportqueueentry-info/

Revision history for this message
Curtis Hovey (sinzui) wrote :

This is fine to land. This is not the first form to but seondary, and possibly distracting information below the form. I had a minor concern that the use of un restrained left and right columns produces a large gutter of white-space between them. I do not see this since I keep my browser narrow, and U do not think you rosetta admins care.

We did discussed two webkit issues that are other bugs: the odd white space between form and buttons and the fact that this page is unreachable in webkit because the edit icons do not render (the fix is already in progress).

review: Approve (ui)
Revision history for this message
Curtis Hovey (sinzui) wrote :

The broken icon is because there is space between the anchor and the span in translation-import-queue-macros.pt. This fixes the issue:
                    <tal:block condition="entry/required:launchpad.Admin">
                      <a class="sprite edit"
                         tal:attributes="href entry/fmt:url"><span class="invisible-link">Change
                         this entry</span></a>
                    </tal:block>

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/translations/browser/configure.zcml'
2--- lib/lp/translations/browser/configure.zcml 2010-01-20 20:36:13 +0000
3+++ lib/lp/translations/browser/configure.zcml 2010-02-11 13:21:26 +0000
4@@ -137,6 +137,7 @@
5 permission="zope.Public"
6 layer="canonical.launchpad.layers.TranslationsLayer"
7 name="+portlet-details"
8+ class="lp.translations.browser.translationimportqueue.TranslationImportQueueEntryView"
9 template="../templates/translationimportqueueentry-portlet-details.pt"/>
10 <browser:url
11 for="lp.translations.interfaces.translationimportqueue.ITranslationImportQueue"
12
13=== added file 'lib/lp/translations/browser/tests/test_translationimportqueueentry.py'
14--- lib/lp/translations/browser/tests/test_translationimportqueueentry.py 1970-01-01 00:00:00 +0000
15+++ lib/lp/translations/browser/tests/test_translationimportqueueentry.py 2010-02-11 13:21:26 +0000
16@@ -0,0 +1,164 @@
17+# Copyright 2010 Canonical Ltd. This software is licensed under the
18+# GNU Affero General Public License version 3 (see the file LICENSE).
19+
20+"""Unit tests for translation import queue views."""
21+
22+from datetime import datetime
23+from pytz import timezone
24+from unittest import TestLoader
25+
26+from zope.component import getMultiAdapter, getUtility
27+from zope.security.proxy import removeSecurityProxy
28+
29+from canonical.testing import LaunchpadFunctionalLayer
30+
31+from canonical.launchpad.layers import TranslationsLayer, setFirstLayer
32+from canonical.launchpad.webapp import canonical_url
33+from canonical.launchpad.webapp.servers import LaunchpadTestRequest
34+from lp.testing import TestCaseWithFactory
35+from lp.translations.interfaces.translationimportqueue import (
36+ ITranslationImportQueue)
37+
38+
39+class TestTranslationImportQueueEntryView(TestCaseWithFactory):
40+ """Tests for the queue entry review form."""
41+
42+ layer = LaunchpadFunctionalLayer
43+
44+ def setUp(self):
45+ super(TestTranslationImportQueueEntryView, self).setUp(
46+ 'foo.bar@canonical.com')
47+ self.queue = getUtility(ITranslationImportQueue)
48+ self.uploader = self.factory.makePerson()
49+
50+ def _makeProductSeries(self):
51+ """Set up a product series for a translatable product."""
52+ product = self.factory.makeProduct()
53+ product.official_rosetta = True
54+ return product.getSeries('trunk')
55+
56+ def _makeView(self, entry):
57+ """Create view for a queue entry."""
58+ request = LaunchpadTestRequest()
59+ setFirstLayer(request, TranslationsLayer)
60+ view = getMultiAdapter((entry, request), name='+index')
61+ view.initialize()
62+ return view
63+
64+ def _makeEntry(self, productseries=None, distroseries=None,
65+ sourcepackagename=None):
66+ filename = self.factory.getUniqueString() + '.pot'
67+ contents = self.factory.getUniqueString()
68+ entry = self.queue.addOrUpdateEntry(
69+ filename, contents, False, self.uploader,
70+ productseries=productseries, distroseries=distroseries,
71+ sourcepackagename=sourcepackagename)
72+ return removeSecurityProxy(entry)
73+
74+ def test_import_target_productseries(self):
75+ # If the entry's attached to a ProductSeries, that's what
76+ # import_target returns.
77+ series = self._makeProductSeries()
78+ entry = self._makeEntry(productseries=series)
79+ view = self._makeView(entry)
80+
81+ self.assertEqual(series, view.import_target)
82+
83+ def test_import_target_sourcepackage(self):
84+ # If the entry has a DistroSeries and a SourcePackageName, the
85+ # import_target is the corresponding SourcePackage.
86+ series = self.factory.makeDistroSeries()
87+ packagename = self.factory.makeSourcePackageName()
88+ package = self.factory.makeSourcePackage(packagename, series)
89+ entry = self._makeEntry(
90+ distroseries=series, sourcepackagename=packagename)
91+ view = self._makeView(entry)
92+
93+ self.assertEqual(package, view.import_target)
94+
95+ def test_productseries_templates_link(self):
96+ # productseries_templates_link counts and, if appropriate links
97+ # to, the series' templates.
98+ series = self._makeProductSeries()
99+ entry = self._makeEntry(productseries=series)
100+ view = self._makeView(entry)
101+
102+ # If there are no templates, there is no link.
103+ self.assertEqual("no templates", view.productseries_templates_link)
104+
105+ # For one template, there is a link. Its text uses the
106+ # singular.
107+ self.factory.makePOTemplate(productseries=series)
108+ self.assertIn('1 template', view.productseries_templates_link)
109+ self.assertNotIn('1 templates', view.productseries_templates_link)
110+ url = canonical_url(series, rootsite='translations') + '/+templates'
111+ self.assertIn(url, view.productseries_templates_link)
112+
113+ def test_product_translatable_series(self):
114+ # If the entry belongs to a productseries, product_translatable_series
115+ # lists the product's translatable series.
116+ series = self._makeProductSeries()
117+ product = series.product
118+ entry = self._makeEntry(productseries=series)
119+ view = self._makeView(entry)
120+
121+ # No translatable series.
122+ series_text = view.product_translatable_series
123+ self.assertEqual("Project has no translatable series.", series_text)
124+
125+ # One translatable series.
126+ extra_series = self.factory.makeProductSeries(product=product)
127+ self.factory.makePOTemplate(productseries=extra_series)
128+ series_text = view.product_translatable_series
129+ self.assertIn("Project has translatable series:", series_text)
130+ # A link follows, and the sentence ends in a period.
131+ self.assertEqual('</a>.', series_text[-5:])
132+
133+ # Two translatable series.
134+ extra_series = self.factory.makeProductSeries(product=product)
135+ self.factory.makePOTemplate(productseries=extra_series)
136+ series_text = view.product_translatable_series
137+ # The links to the series are separated by a comma.
138+ self.assertIn("</a>, <a ", series_text)
139+ # The sentence ends in a period.
140+ self.assertEqual('</a>.', series_text[-5:])
141+
142+ # Many translatable series. The list is cut short; there's an
143+ # ellipsis to indicate this.
144+ series_count = len(product.translatable_series)
145+ for counter in xrange(series_count, view.max_series_to_display + 1):
146+ extra_series = self.factory.makeProductSeries(product=product)
147+ self.factory.makePOTemplate(productseries=extra_series)
148+ series_text = view.product_translatable_series
149+ # The list is cut short.
150+ displayed_series_count = series_text.count('</a>')
151+ self.assertNotEqual(
152+ len(product.translatable_series), displayed_series_count)
153+ self.assertEqual(view.max_series_to_display, displayed_series_count)
154+ # The list of links ends with an ellipsis.
155+ self.assertEqual('</a>, ...', series_text[-9:])
156+
157+ def test_status_change_date(self):
158+ # status_change_date describes the date of the entry's last
159+ # status change.
160+ series = self._makeProductSeries()
161+ product = series.product
162+ entry = self._makeEntry(productseries=series)
163+ view = self._makeView(entry)
164+
165+ # If the date equals the upload date, there's no need to show
166+ # anything.
167+ self.assertEqual('', view.status_change_date)
168+
169+ # If there is a difference, there's a human-readable
170+ # description.
171+ UTC = timezone('UTC')
172+ entry.dateimported = datetime(year=2005, month=11, day=29, tzinfo=UTC)
173+ entry.date_status_changed = datetime(
174+ year=2007, month=8, day=14, tzinfo=UTC)
175+ self.assertEqual(
176+ "Last changed on 2007-08-14.", view.status_change_date)
177+
178+
179+def test_suite():
180+ return TestLoader().loadTestsFromName(__name__)
181
182=== modified file 'lib/lp/translations/browser/translationimportqueue.py'
183--- lib/lp/translations/browser/translationimportqueue.py 2010-01-13 04:46:11 +0000
184+++ lib/lp/translations/browser/translationimportqueue.py 2010-02-11 13:21:26 +0000
185@@ -1,7 +1,7 @@
186-# Copyright 2009 Canonical Ltd. This software is licensed under the
187+# Copyright 2009-2010 Canonical Ltd. This software is licensed under the
188 # GNU Affero General Public License version 3 (see the file LICENSE).
189
190-"""Browser views for ITranslationImportQueue."""
191+"""Browser views for `ITranslationImportQueue`."""
192
193 __metaclass__ = type
194
195@@ -21,9 +21,13 @@
196 from zope.schema.vocabulary import SimpleVocabulary, SimpleTerm
197
198 from canonical.database.constants import UTC_NOW
199+from canonical.launchpad.webapp.tales import DateTimeFormatterAPI
200+from canonical.launchpad.webapp.interfaces import (
201+ NotFoundError, UnexpectedFormData)
202 from lp.translations.browser.hastranslationimports import (
203 HasTranslationImportsView)
204 from lp.registry.interfaces.distroseries import IDistroSeries
205+from lp.registry.interfaces.sourcepackage import ISourcePackageFactory
206 from lp.translations.interfaces.translationimportqueue import (
207 ITranslationImportQueueEntry, IEditTranslationImportQueueEntry,
208 ITranslationImportQueue, RosettaImportStatus,
209@@ -31,8 +35,6 @@
210 from lp.services.worlddata.interfaces.language import ILanguageSet
211 from lp.translations.interfaces.pofile import IPOFileSet
212 from lp.translations.interfaces.potemplate import IPOTemplateSet
213-from canonical.launchpad.webapp.interfaces import (
214- NotFoundError, UnexpectedFormData)
215
216 from canonical.launchpad.webapp import (
217 action, canonical_url, GetitemNavigation, LaunchpadFormView)
218@@ -48,6 +50,8 @@
219 label = "Review import queue entry"
220 schema = IEditTranslationImportQueueEntry
221
222+ max_series_to_display = 3
223+
224 @property
225 def initial_values(self):
226 """Initialize some values on the form, when it's possible."""
227@@ -129,6 +133,84 @@
228 return None
229
230 @property
231+ def import_target(self):
232+ """The entry's `ProductSeries` or `SourcePackage`."""
233+ productseries = self.context.productseries
234+ distroseries = self.context.distroseries
235+ sourcepackagename = self.context.sourcepackagename
236+ if distroseries is None:
237+ return productseries
238+ else:
239+ factory = getUtility(ISourcePackageFactory)
240+ return factory.new(sourcepackagename, distroseries)
241+
242+ @property
243+ def productseries_templates_link(self):
244+ """Return link to `ProductSeries`' templates.
245+
246+ Use this only if the entry is attached to a `ProductSeries`.
247+ """
248+ assert self.context.productseries is not None, (
249+ "Entry is not attached to a ProductSeries.")
250+
251+ template_count = self.context.productseries.potemplate_count
252+ if template_count == 0:
253+ return "no templates"
254+ else:
255+ link = "%s/+templates" % canonical_url(
256+ self.context.productseries, rootsite='translations')
257+ if template_count == 1:
258+ word = "template"
259+ else:
260+ word = "templates"
261+ return '<a href="%s">%d %s</a>' % (link, template_count, word)
262+
263+ def _composeProductSeriesLink(self, productseries):
264+ """Produce HTML to link to `productseries`."""
265+ return '<a href="%s">%s</a>' % (
266+ canonical_url(productseries, rootsite='translations'),
267+ productseries.name)
268+
269+ @property
270+ def product_translatable_series(self):
271+ """Summarize whether `Product` has translatable series.
272+
273+ Use this only if the entry is attached to a `ProductSeries`.
274+ """
275+ assert self.context.productseries is not None, (
276+ "Entry is not attached to a ProductSeries.")
277+
278+ product = self.context.productseries.product
279+ translatable_series = list(product.translatable_series)
280+ if len(translatable_series) == 0:
281+ return "Project has no translatable series."
282+ else:
283+ links = [
284+ self._composeProductSeriesLink(series)
285+ for series in translatable_series[:self.max_series_to_display]
286+ ]
287+ links_text = ', '.join(links)
288+ if len(translatable_series) > self.max_series_to_display:
289+ tail = ", ..."
290+ else:
291+ tail = "."
292+ return "Project has translatable series: " + links_text + tail
293+
294+ @property
295+ def status_change_date(self):
296+ """Show date of last status change.
297+
298+ Says nothing at all if the entry's status has not changed since
299+ upload.
300+ """
301+ change_date = self.context.date_status_changed
302+ if change_date == self.context.dateimported:
303+ return ""
304+ else:
305+ formatter = DateTimeFormatterAPI(change_date)
306+ return "Last changed %s." % formatter.displaydate()
307+
308+ @property
309 def next_url(self):
310 """See `LaunchpadFormView`."""
311 # The referer header we want is only available before the view's
312
313=== added file 'lib/lp/translations/stories/importqueue/xx-entry-details.txt'
314--- lib/lp/translations/stories/importqueue/xx-entry-details.txt 1970-01-01 00:00:00 +0000
315+++ lib/lp/translations/stories/importqueue/xx-entry-details.txt 2010-02-11 13:21:26 +0000
316@@ -0,0 +1,106 @@
317+= Entry details =
318+
319+The translation import queue entry page shows various details about an
320+entry and its target that may be helpful in queue review.
321+
322+ >>> from zope.security.proxy import removeSecurityProxy
323+ >>> from lp.translations.model.translationimportqueue import (
324+ ... TranslationImportQueue)
325+
326+ >>> filename = 'po/foo.pot'
327+
328+ >>> login(ANONYMOUS)
329+ >>> queue = TranslationImportQueue()
330+ >>> product = factory.makeProduct()
331+ >>> removeSecurityProxy(product).official_rosetta = True
332+ >>> trunk = product.getSeries('trunk')
333+ >>> uploader = factory.makePerson()
334+ >>> entry = queue.addOrUpdateEntry(
335+ ... filename, '# empty', False, uploader, productseries=trunk)
336+ >>> entry_url = canonical_url(entry, rootsite='translations')
337+ >>> logout()
338+
339+ >>> admin_browser.open(entry_url)
340+ >>> details = find_tag_by_id(admin_browser.contents, 'portlet-details')
341+ >>> details_text = extract_text(details)
342+ >>> print details_text
343+ Upload attached to ... trunk series.
344+ This project...s license is open source.
345+ Release series has no templates.
346+ Project has no translatable series.
347+ File po/foo.pot uploaded by ...
348+
349+The details include the project the entry is for, and who uploaded it.
350+
351+ >>> product.displayname in details_text
352+ True
353+ >>> uploader.displayname in details_text
354+ True
355+
356+There's also a link to the file's contents.
357+
358+ >>> print admin_browser.getLink(filename).text
359+ po/foo.pot
360+ >>> print admin_browser.getLink(filename).url
361+ http://...foo.pot
362+
363+
364+== Existing templates ==
365+
366+If there are translatable templates in the series, this will be stated
367+and there will be a link to the templates list.
368+
369+ >>> login(ANONYMOUS)
370+ >>> template = factory.makePOTemplate(productseries=trunk)
371+ >>> logout()
372+
373+ >>> admin_browser.open(entry_url)
374+ >>> details = find_tag_by_id(admin_browser.contents, 'portlet-details')
375+ >>> details_text = extract_text(details)
376+ >>> print details_text
377+ Upload attached to
378+ ...
379+ Release series has 1 template.
380+ ...
381+
382+ >>> print admin_browser.getLink('1 template').url
383+ http...://translations.launchpad.dev/.../trunk/+templates
384+
385+ >>> admin_browser.getLink('1 template').click()
386+ >>> print admin_browser.title
387+ All templates : Translations : Series trunk : ...
388+
389+In that case, the product is also shown to have translatable series.
390+
391+ >>> print details_text
392+ Upload attached to
393+ ...
394+ Project has translatable series: trunk.
395+ ...
396+
397+
398+== Source packages ==
399+
400+The portlet shows different (well, less) information for uploads
401+attached to distribution packages.
402+
403+ >>> from lp.registry.model.distribution import DistributionSet
404+
405+ >>> login(ANONYMOUS)
406+ >>> distro = DistributionSet().getByName('ubuntu')
407+ >>> distroseries = distro.getSeries('hoary')
408+ >>> packagename = factory.makeSourcePackageName(name='xpad')
409+ >>> entry = queue.addOrUpdateEntry(
410+ ... filename, '# nothing', True, uploader, distroseries=distroseries,
411+ ... sourcepackagename=packagename)
412+ >>> entry_url = canonical_url(entry, rootsite='translations')
413+ >>> logout()
414+
415+This only shows the source package and the file.
416+
417+ >>> admin_browser.open(entry_url)
418+ >>> details = find_tag_by_id(admin_browser.contents, 'portlet-details')
419+ >>> details_text = extract_text(details)
420+ >>> print details_text
421+ Upload attached to xpad in Ubuntu Hoary.
422+ File po/foo.pot uploaded by ...
423
424=== added file 'lib/lp/translations/stories/importqueue/xx-entry-error-output.txt'
425--- lib/lp/translations/stories/importqueue/xx-entry-error-output.txt 1970-01-01 00:00:00 +0000
426+++ lib/lp/translations/stories/importqueue/xx-entry-error-output.txt 2010-02-11 13:21:26 +0000
427@@ -0,0 +1,46 @@
428+= Translation import queue entry error output =
429+
430+The approval form translation import queue entries shows any error
431+or warning output that the entry may have incurred.
432+
433+By default, this is nothing.
434+
435+ >>> from lp.translations.model.translationimportqueue import (
436+ ... TranslationImportQueue)
437+
438+ >>> def find_error_output(browser):
439+ ... """Find error-output section on page."""
440+ ... return find_tag_by_id(browser.contents, 'error-output')
441+
442+ >>> login(ANONYMOUS)
443+ >>> queue = TranslationImportQueue()
444+ >>> product = factory.makeProduct()
445+ >>> trunk = product.getSeries('trunk')
446+ >>> entry = queue.addOrUpdateEntry(
447+ ... 'la.po', '# contents', False, product.owner, productseries=trunk)
448+ >>> entry_url = canonical_url(entry, rootsite='translations')
449+ >>> logout()
450+
451+ >>> admin_browser.open(entry_url)
452+ >>> output_panel = find_error_output(admin_browser)
453+ >>> print output_panel
454+ None
455+
456+The section showing the output only shows up when there is output to
457+show.
458+
459+ >>> entry.error_output = "Things went horribly wrong."
460+ >>> admin_browser.open(entry_url)
461+ >>> output_panel = find_error_output(admin_browser)
462+ >>> print extract_text(output_panel)
463+ Error output for this entry:
464+ Things went horribly wrong.
465+
466+The output is properly HTML-escaped, so is safe to display in this way.
467+
468+ >>> entry.error_output = "<h1>Injection &amp; subterfuge</h1>"
469+ >>> admin_browser.open(entry_url)
470+ >>> output_panel = find_error_output(admin_browser)
471+ >>> print output_panel.renderContents()
472+ Error output for this entry:
473+ ...&lt;h1&gt;Injection &amp;amp; subterfuge&lt;/h1&gt;...
474
475=== modified file 'lib/lp/translations/templates/translationimportqueueentry-index.pt'
476--- lib/lp/translations/templates/translationimportqueueentry-index.pt 2009-12-03 18:33:22 +0000
477+++ lib/lp/translations/templates/translationimportqueueentry-index.pt 2010-02-11 13:21:26 +0000
478@@ -106,6 +106,19 @@
479 tal:attributes="value view/referrer_url" />
480 </div>
481 </div>
482+
483+ <tal:details replace="structure context/@@+portlet-details" />
484+
485+ <div tal:condition="context/error_output"
486+ class="portlet"
487+ id="error-output">
488+ Error output for this entry:
489+ <br />
490+ <tt tal:content="context/error_output">
491+ Horrible syntax error near line 192.
492+ </tt>
493+ </div>
494+
495 </div>
496
497 </body>
498
499=== modified file 'lib/lp/translations/templates/translationimportqueueentry-portlet-details.pt'
500--- lib/lp/translations/templates/translationimportqueueentry-portlet-details.pt 2009-07-17 17:59:07 +0000
501+++ lib/lp/translations/templates/translationimportqueueentry-portlet-details.pt 2010-02-11 13:21:26 +0000
502@@ -4,41 +4,48 @@
503 xmlns:i18n="http://xml.zope.org/namespaces/i18n"
504 omit-tag="">
505
506-<div class="portlet" id="portlet-details">
507- <h2>Import entry details</h2>
508-
509- <div class="portletBody">
510- <div class="portletContent">
511- <b>Origin:</b><br />
512- <a tal:condition="context/sourcepackage"
513- tal:content="context/sourcepackage/title"
514- tal:attributes="href string:${context/sourcepackage/fmt:url}"
515- >evolution in Ubuntu Hoary</a>
516- <a tal:condition="context/productseries"
517- tal:content="context/productseries/title"
518- tal:attributes="href string:${context/productseries/fmt:url}"
519- >Evolution Series: MAIN</a><br />
520- <b>Path:</b>
521- <a tal:attributes="href context/content/http_url"
522- tal:content="context/path">po/foo.pot</a><br />
523- <b>Uploader:</b><br />
524- <a tal:replace="structure context/importer/fmt:link"
525- >Mark Shuttleworth</a><br />
526- <b>Waiting since:</b>
527- <span
528- tal:attributes="title context/dateimported/fmt:datetime"
529- tal:content="context/dateimported/fmt:approximatedate">
530- 2004-05-22
531- </span><br />
532- <b>Status:</b>
533- <span tal:replace="context/status/title"></span><br />
534- <b>Status changed:</b>
535- <span
536- tal:attributes="title context/date_status_changed/fmt:datetime"
537- tal:content="context/date_status_changed/fmt:approximatedate">
538- 2004-05-21
539- </span><br />
540- </div>
541- </div>
542+<div class="columns portlet" id="portlet-details">
543+ <div class="two column left">
544+ Upload attached to
545+ <a tal:replace="structure view/import_target/fmt:link">
546+ Evolution in Ubuntu Hoary</a>.
547+
548+ <tal:productseries condition="context/productseries">
549+ <ul class="bulleted" tal:define="product context/productseries/product">
550+ <li>
551+ <tal:license
552+ replace="structure product/license_status/description">
553+ This project's license is open source.
554+ </tal:license>
555+ </li>
556+ <li>
557+ Release series has
558+ <tal:templates replace="structure view/productseries_templates_link">
559+ 2 templates</tal:templates>.
560+ </li>
561+ <li>
562+ <tal:series replace="structure view/product_translatable_series">
563+ Project has translatable series: trunk, 0.1, 0.2, ...
564+ </tal:series>
565+ </li>
566+ </ul>
567+ </tal:productseries>
568+ </div>
569+
570+ <div class="two column right">
571+ File
572+ <a tal:attributes="href context/content/http_url"
573+ tal:content="context/path">po/messages.pot</a>
574+ uploaded by
575+ <a tal:replace="structure context/importer/fmt:link">
576+ Arne Goetje
577+ </a>
578+ <tal:upload_date replace="context/dateimported/fmt:displaydate">
579+ 2010-02-10
580+ </tal:upload_date>.
581+ <tal:status_change replace="view/status_change_date">
582+ Entry last changed on 2010-02-12.
583+ </tal:status_change>
584+ </div>
585 </div>
586 </tal:root>