Launchpad itself

Merge lp:~jtv/launchpad/bug-618393 into lp:launchpad

  1. bug-618393
  2. Merge into devel
Proposed by Jeroen T. Vermeulen
Status: Merged
Approved by: Jeroen T. Vermeulen
Approved revision: no longer in the source branch.
Merged at revision: 11498
Proposed branch: lp:~jtv/launchpad/bug-618393
Merge into: lp:launchpad
Diff against target: 804 lines (+361/-103)
10 files modified
lib/lp/registry/model/pillar.py (+7/-27)
lib/lp/registry/model/product.py (+49/-5)
lib/lp/registry/tests/test_productwithlicenses.py (+114/-0)
lib/lp/translations/browser/tests/test_translationgroup.py (+2/-6)
lib/lp/translations/browser/translationgroup.py (+4/-3)
lib/lp/translations/doc/translationgroup.txt (+9/-7)
lib/lp/translations/interfaces/translationgroup.py (+28/-2)
lib/lp/translations/model/translationgroup.py (+114/-21)
lib/lp/translations/templates/translationgroup-index.pt (+17/-16)
lib/lp/translations/templates/translationgroup-portlet-projects.pt (+17/-16)
To merge this branch: bzr merge lp:~jtv/launchpad/bug-618393
Reviewer Review Type Date Requested Status
Henning Eggers (community) code Approve
Review via email: mp+34515@code.launchpad.net

Commit message

TranslationGroup:+index optimization, ProductWithLicenses cleanup

Description of the change

= Bug 618393 =

Working on some timeouts on the TranslationGroup pages.

I looked at one oops in detail. A narrow majority of the time was taken up by database queries. So I mostly attacked those before resorting to python profiling.

Here I exploit several optimization opportunities:
 * Prejoin icon data for persons, persons, etc. (20% of queries).
 * Prejoin project license information (15% of queries).
 * Cache whether user has Edit privileges in the view.
 * Query translators once; avoid redundant is_empty check.
 * Query for projects/project groups/distributions on the slave store.

The most interesting work is on prefetching project license information. What makes this necessary is the link formatter for Product: it queries for licenses in order to flag projects whose licenses have not yet been set.

These are not queries one can avoid through Storm caching. Luckily there is a class that lets me get around that: ProductWithLicenses. It's a wrapper for Product that you can pre-seed with the project's licenses, which you can obtain from the same query that gave you the Product itself. This helper is still a bit rough around the edges; I added a class method to compose the necessary join in Storm and took some of the data massage from the call site into the constructor.

You'll notice that I had to change the license_status property in ProductWithLicenses. The reason is a subtle mismatch in access permissions between Product and ProductWithLicenses: license_status is publicly visible either way, but it is implemented in terms of license_approved and license_reviewed which require special privileges. The Product implementation got around this trivially: license_status is public, and once you're in the property code, you're past the security proxy and can access self.license_approved etc. without problems. But the ProductWithLicenses implementation is an outsider: when it evaluates self.license_approved it delegates to self.product, which is still shielded by a security proxy. Thus ProductWithLicenses de facto required special privileges. I bypassed the proxy for this, exposing license_status but not license_approved—exactly as in Product.

To test,
{{{
./bin/test -vvc lp.translations -t translationgroup
./bin/test -vvc lp.registry -t productwithlicenses -t pillar.txt
}}}

To Q/A, go to

    https://translations.edge.launchpad.net/+groups/ubuntu-translators/+index

It should not time out. Also, the page should issue at most 600 or so queries, as opposed to the current 1,000.

No lint,

Jeroen

To post a comment you must log in.
Revision history for this message
Henning Eggers (henninge) wrote :
Download full text (12.0 KiB)

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

Hi Jeroen,
very nice branch and I learned a few new things by reviewing it. Thanks. ;-)

I could not find anything serious so please bear my nit-picks with patience.
And thank you for all the drive-by fixes! ;)

 review approve code

Cheers,
Henning

Am 03.09.2010 08:46, schrieb Jeroen T. Vermeulen:
> === modified file 'lib/lp/registry/model/pillar.py'
> @@ -244,18 +230,12 @@
> stacklevel=2)
> pillars = []
> # Prefill pillar.product.licenses.
> - for pillar_name, other, product, project, distro, license_ids in (
> + for pillar_name, other, product, project, distro, licenses in (
> result[:limit]):

You can fix this confusing indention by pulling the de-tupling into the loop body.

> pillar = pillar_name.pillar
> if IProduct.providedBy(pillar):
> - licenses = [
> - License.items[license_id]
> - for license_id in license_ids]
> - product_with_licenses = ProductWithLicenses(
> - pillar, tuple(sorted(licenses)))
> - pillars.append(product_with_licenses)
> - else:
> - pillars.append(pillar)
> + pillar = ProductWithLicenses(pillar, licenses)
> + pillars.append(pillar)
> return pillars
>
> def add_featured_project(self, project):
>
> === modified file 'lib/lp/registry/model/product.py'
> @@ -201,14 +202,25 @@
> return LicenseStatus.OPEN_SOURCE
>
>
> +class Array(NamedFunc):
> + """Implements the postgres "array" function in Storm."""
> + name = 'array'
> +
> +
> class ProductWithLicenses:
> """Caches `Product.licenses`."""
>
> delegates(IProduct, 'product')
>
> - def __init__(self, product, licenses):
> + def __init__(self, product, license_ids):
> + """Initialize a `ProductWithLicenses`.
> +
> + :param product: the `Product` to wrap.
> + :param license_ids: a sequence of numeric `License` ids.
> + """
> self.product = product
> - self._licenses = licenses
> + self._licenses = tuple([
> + License.items[id] for id in sorted(license_ids)])

You don't need the [] inside the tuple(). Go straight from iterator to tuple.
(I just found that out recently myself.)

>
> @property
> def licenses(self):

> === modified file 'lib/lp/translations/interfaces/translationgroup.py'
> --- lib/lp/translations/interfaces/translationgroup.py 2010-08-20 20:31:18 +0000
> +++ lib/lp/translations/interfaces/translationgroup.py 2010-09-03 06:46:46 +0000
> @@ -186,8 +186,34 @@
> def fetchTranslatorData():
> """Fetch translators and related data.
>
> - :return: A tuple (`Translator`, `Language`, `Person`), ordered
> - by language name in English.
> + Prefetches display-related properties.
> +
> + :return: A result set of (`Translator`, `Language`, `Person`),
> + ordered by language name in English.
> + """
> +
> + def fetchProjectsForDisplay():

I would ask you to use "fetchProductsForDisplay" as l...

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

Hi Jeroen,
very nice branch and I learned a few new things by reviewing it. Thanks. ;-)

I could not find anything serious so please bear my nit-picks with patience.
And thank you for all the drive-by fixes! ;)

Cheers,
Henning

Am 03.09.2010 08:46, schrieb Jeroen T. Vermeulen:
> > === modified file 'lib/lp/registry/model/pillar.py'
> > @@ -244,18 +230,12 @@
> > stacklevel=2)
> > pillars = []
> > # Prefill pillar.product.licenses.
> > - for pillar_name, other, product, project, distro, license_ids in (
> > + for pillar_name, other, product, project, distro, licenses in (
> > result[:limit]):
 You can fix this confusing indention by pulling the de-tupling into the loop body.

> > pillar = pillar_name.pillar
> > if IProduct.providedBy(pillar):
> > - licenses = [
> > - License.items[license_id]
> > - for license_id in license_ids]
> > - product_with_licenses = ProductWithLicenses(
> > - pillar, tuple(sorted(licenses)))
> > - pillars.append(product_with_licenses)
> > - else:
> > - pillars.append(pillar)
> > + pillar = ProductWithLicenses(pillar, licenses)
> > + pillars.append(pillar)
> > return pillars
> >
> > def add_featured_project(self, project):
> >
> > === modified file 'lib/lp/registry/model/product.py'
> > @@ -201,14 +202,25 @@
> > return LicenseStatus.OPEN_SOURCE
> >
> >
> > +class Array(NamedFunc):
> > + """Implements the postgres "array" function in Storm."""
> > + name = 'array'
> > +
> > +
> > class ProductWithLicenses:
> > """Caches `Product.licenses`."""
> >
> > delegates(IProduct, 'product')
> >
> > - def __init__(self, product, licenses):
> > + def __init__(self, product, license_ids):
> > + """Initialize a `ProductWithLicenses`.
> > +
> > + :param product: the `Product` to wrap.
> > + :param license_ids: a sequence of numeric `License` ids.
> > + """
> > self.product = product
> > - self._licenses = licenses
> > + self._licenses = tuple([
> > + License.items[id] for id in sorted(license_ids)])
You don't need the [] inside the tuple(). Go straight from iterator to tuple.
(I just found that out recently myself.)

> >
> > @property
> > def licenses(self):
> > === modified file 'lib/lp/translations/interfaces/translationgroup.py'
> > --- lib/lp/translations/interfaces/translationgroup.py 2010-08-20 20:31:18 +0000
> > +++ lib/lp/translations/interfaces/translationgroup.py 2010-09-03 06:46:46 +0000
> > @@ -186,8 +186,34 @@
> > def fetchTranslatorData():
> > """Fetch translators and related data.
> >
> > - :return: A tuple (`Translator`, `Language`, `Person`), ordered
> > - by language name in English.
> > + Prefetches display-related properties.
> > +
> > + :return: A result set of (`Translator`, `Language`, `Person`),
> > + ordered by language name in English.
> > + """
> > +
> > + def fetchProjectsForDisplay():
I w...

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

Thanks. Wow, you did some careful research!

All your suggestions implemented, except moving the order_by out of the prejoin. (If nothing else, I like to avoid any risk of triggering query-before-slice bugs a priori). I didn't know tuple comprehensions were in python 2.5; I thought they were 2.6-only and so didn't use them.

You're also right that I shouldn't have said "Project" for "Product" in model code. It was reckless of me, and it is now undone.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/registry/model/pillar.py'
2--- lib/lp/registry/model/pillar.py 2010-08-20 20:31:18 +0000
3+++ lib/lp/registry/model/pillar.py 2010-09-03 11:18:46 +0000
4@@ -17,11 +17,7 @@
5 ForeignKey,
6 StringCol,
7 )
8-from storm.expr import (
9- LeftJoin,
10- NamedFunc,
11- Select,
12- )
13+from storm.expr import LeftJoin
14 from storm.info import ClassAlias
15 from storm.locals import SQL
16 from storm.store import Store
17@@ -51,11 +47,9 @@
18 from lp.registry.interfaces.product import (
19 IProduct,
20 IProductSet,
21- License,
22 )
23 from lp.registry.interfaces.projectgroup import IProjectGroupSet
24 from lp.registry.model.featuredproject import FeaturedProject
25-from lp.registry.model.productlicense import ProductLicense
26
27
28 __all__ = [
29@@ -197,23 +191,15 @@
30
31 def search(self, text, limit):
32 """See `IPillarSet`."""
33- # These classes are imported in this method to prevent an import loop.
34- from lp.registry.model.product import (
35- Product, ProductWithLicenses)
36+ # Avoid circular import.
37+ from lp.registry.model.product import ProductWithLicenses
38 if limit is None:
39 limit = config.launchpad.default_batch_size
40
41- class Array(NamedFunc):
42- """Storm representation of the array() PostgreSQL function."""
43- name = 'array'
44-
45 # Pull out the licenses as a subselect which is converted
46 # into a PostgreSQL array so that multiple licenses per product
47 # can be retrieved in a single row for each product.
48- extra_column = Array(
49- Select(columns=[ProductLicense.license],
50- where=(ProductLicense.product == Product.id),
51- tables=[ProductLicense]))
52+ extra_column = ProductWithLicenses.composeLicensesColumn()
53 result = self.build_search_query(text, [extra_column])
54
55 # If the search text matches the name or title of the
56@@ -244,18 +230,12 @@
57 stacklevel=2)
58 pillars = []
59 # Prefill pillar.product.licenses.
60- for pillar_name, other, product, project, distro, license_ids in (
61+ for pillar_name, other, product, project, distro, licenses in (
62 result[:limit]):
63 pillar = pillar_name.pillar
64 if IProduct.providedBy(pillar):
65- licenses = [
66- License.items[license_id]
67- for license_id in license_ids]
68- product_with_licenses = ProductWithLicenses(
69- pillar, tuple(sorted(licenses)))
70- pillars.append(product_with_licenses)
71- else:
72- pillars.append(pillar)
73+ pillar = ProductWithLicenses(pillar, licenses)
74+ pillars.append(pillar)
75 return pillars
76
77 def add_featured_project(self, project):
78
79=== modified file 'lib/lp/registry/model/product.py'
80--- lib/lp/registry/model/product.py 2010-09-03 03:12:39 +0000
81+++ lib/lp/registry/model/product.py 2010-09-03 11:18:46 +0000
82@@ -1,4 +1,4 @@
83-# Copyright 2009 Canonical Ltd. This software is licensed under the
84+# Copyright 2009-2010 Canonical Ltd. This software is licensed under the
85 # GNU Affero General Public License version 3 (see the file LICENSE).
86 # pylint: disable-msg=E0611,W0212
87
88@@ -25,11 +25,13 @@
89 SQLObjectNotFound,
90 StringCol,
91 )
92+from storm.expr import NamedFunc
93 from storm.locals import (
94 And,
95 Desc,
96 Int,
97 Join,
98+ Select,
99 SQL,
100 Store,
101 Unicode,
102@@ -85,7 +87,6 @@
103 SpecificationDefinitionStatus,
104 SpecificationFilter,
105 SpecificationImplementationStatus,
106- SpecificationSort,
107 )
108 from lp.blueprints.model.specification import (
109 HasSpecificationsMixin,
110@@ -201,14 +202,25 @@
111 return LicenseStatus.OPEN_SOURCE
112
113
114+class Array(NamedFunc):
115+ """Implements the postgres "array" function in Storm."""
116+ name = 'array'
117+
118+
119 class ProductWithLicenses:
120 """Caches `Product.licenses`."""
121
122 delegates(IProduct, 'product')
123
124- def __init__(self, product, licenses):
125+ def __init__(self, product, license_ids):
126+ """Initialize a `ProductWithLicenses`.
127+
128+ :param product: the `Product` to wrap.
129+ :param license_ids: a sequence of numeric `License` ids.
130+ """
131 self.product = product
132- self._licenses = licenses
133+ self._licenses = tuple([
134+ License.items[id] for id in sorted(license_ids)])
135
136 @property
137 def licenses(self):
138@@ -223,8 +235,40 @@
139 `Product.licenses`, which is not cached, instead of
140 `ProductWithLicenses.licenses`, which is cached.
141 """
142+ naked_product = removeSecurityProxy(self.product)
143 return get_license_status(
144- self.license_approved, self.license_reviewed, self.licenses)
145+ naked_product.license_approved, naked_product.license_reviewed,
146+ self.licenses)
147+
148+ @classmethod
149+ def composeLicensesColumn(cls, for_class=None):
150+ """Compose a Storm column specification for licenses.
151+
152+ Use this to render a list of `Product` linkes without querying
153+ licenses for each one individually.
154+
155+ It lets you prefetch the licensing information in the same
156+ query that fetches a `Product`. Just add the column spec
157+ returned by this function to the query, and pass it to the
158+ `ProductWithLicenses` constructor:
159+
160+ license_column = ProductWithLicenses.composeLicensesColumn()
161+ products_with_licenses = [
162+ ProductWithLicenses(product, licenses)
163+ for product, licenses in store.find(Product, license_column)
164+ ]
165+
166+ :param for_class: Class to find licenses for. Defaults to
167+ `Product`, but could also be a Storm `ClassAlias`.
168+ """
169+ if for_class is None:
170+ for_class = Product
171+
172+ return Array(
173+ Select(
174+ columns=[ProductLicense.license],
175+ where=(ProductLicense.product == for_class.id),
176+ tables=[ProductLicense]))
177
178
179 class Product(SQLBase, BugTargetBase, MakesAnnouncements,
180
181=== added file 'lib/lp/registry/tests/test_productwithlicenses.py'
182--- lib/lp/registry/tests/test_productwithlicenses.py 1970-01-01 00:00:00 +0000
183+++ lib/lp/registry/tests/test_productwithlicenses.py 2010-09-03 11:18:46 +0000
184@@ -0,0 +1,114 @@
185+# Copyright 2010 Canonical Ltd. This software is licensed under the
186+# GNU Affero General Public License version 3 (see the file LICENSE).
187+
188+"""Unit tests for `ProductWithLicenses`."""
189+
190+__metaclass__ = type
191+
192+from operator import attrgetter
193+
194+from storm.store import Store
195+from zope.interface.verify import verifyObject
196+
197+from canonical.launchpad.ftests import (
198+ ANONYMOUS,
199+ login,
200+ )
201+from canonical.testing import DatabaseFunctionalLayer
202+from lp.registry.interfaces.product import (
203+ IProduct,
204+ License,
205+ LicenseStatus,
206+ )
207+from lp.registry.model.product import (
208+ Product,
209+ ProductWithLicenses,
210+ )
211+from lp.testing import TestCaseWithFactory
212+
213+
214+class TestProductWithLicenses(TestCaseWithFactory):
215+
216+ layer = DatabaseFunctionalLayer
217+
218+ def test_baseline(self):
219+ product = self.factory.makeProduct()
220+ product_with_licenses = ProductWithLicenses(product, [])
221+ # Log in--a full verification takes Edit privileges.
222+ login('foo.bar@canonical.com')
223+ self.assertTrue(verifyObject(IProduct, product_with_licenses))
224+
225+ def test_uses_cached_licenses(self):
226+ # The ProductWithLicenses' licensing information is based purely
227+ # on the cached licenses list. The database is not queried to
228+ # determine licensing status.
229+ product = self.factory.makeProduct(licenses=[License.BSD])
230+ product_with_licenses = ProductWithLicenses(
231+ product, [License.OTHER_PROPRIETARY.value])
232+ license_status = product_with_licenses.license_status
233+ self.assertEqual(LicenseStatus.PROPRIETARY, license_status)
234+
235+ def test_sorts_licenses(self):
236+ # The ProductWithLicenses constructor sorts the Licenses by
237+ # numeric value.
238+ product = self.factory.makeProduct()
239+ licenses = [License.AFFERO, License.BSD, License.MIT]
240+
241+ # Feed the constructor a list of ids in the wrong order.
242+ product_with_licenses = ProductWithLicenses(
243+ product,
244+ sorted([license.value for license in licenses], reverse=True))
245+
246+ expected = sorted(licenses, key=attrgetter('value'))
247+ self.assertEqual(tuple(expected), product_with_licenses.licenses)
248+
249+ def test_compose_column_without_licenses_produces_empty(self):
250+ # The licenses column that ProductWithLicenses produces for a
251+ # product without licenses contains an empty list.
252+ product = self.factory.makeProduct(licenses=[])
253+ column = ProductWithLicenses.composeLicensesColumn()
254+ store = Store.of(product)
255+ result = list(store.find((Product, column), Product.id == product.id))
256+
257+ self.assertEqual([(product, [])], result)
258+
259+ def test_licenses_column_contains_licensing_info(self):
260+ # Feeding the licenses column into the ProductWithLicenses
261+ # constructor seeds it with the appropriate licenses.
262+ product = self.factory.makeProduct(
263+ licenses=[License.OTHER_PROPRIETARY])
264+ column = ProductWithLicenses.composeLicensesColumn()
265+ store = Store.of(product)
266+ row = store.find((Product, column), Product.id == product.id).one()
267+
268+ product_with_licenses = ProductWithLicenses(*row)
269+ licenses = product_with_licenses.licenses
270+ license_status = product_with_licenses.license_status
271+ self.assertEqual((License.OTHER_PROPRIETARY, ), licenses)
272+ self.assertEqual(LicenseStatus.PROPRIETARY, license_status)
273+
274+ def test_licenses_column_aggregates(self):
275+ # Adding a licensing column for a product with multiple licenses
276+ # still finds a single product, not one per license.
277+ licenses = [License.AFFERO, License.GNU_GPL_V3]
278+ product = self.factory.makeProduct(licenses=licenses)
279+ column = ProductWithLicenses.composeLicensesColumn()
280+ store = Store.of(product)
281+ result = list(store.find((Product, column), Product.id == product.id))
282+
283+ self.assertEqual(1, len(result))
284+ found_product, found_licenses = result[0]
285+ self.assertEqual(product, found_product)
286+ self.assertEqual(len(licenses), len(found_licenses))
287+
288+ def test_license_status_is_public(self):
289+ # The license_status attribute can be read by anyone, on
290+ # ProductWithLicenses as on Product.
291+ product = self.factory.makeProduct(licenses=[License.BSD])
292+ product_with_licenses = ProductWithLicenses(
293+ product, [License.BSD.value])
294+ login(ANONYMOUS)
295+ self.assertEqual(
296+ LicenseStatus.OPEN_SOURCE, product.license_status)
297+ self.assertEqual(
298+ LicenseStatus.OPEN_SOURCE, product_with_licenses.license_status)
299
300=== modified file 'lib/lp/translations/browser/tests/test_translationgroup.py'
301--- lib/lp/translations/browser/tests/test_translationgroup.py 2010-08-20 20:31:18 +0000
302+++ lib/lp/translations/browser/tests/test_translationgroup.py 2010-09-03 11:18:46 +0000
303@@ -5,8 +5,7 @@
304
305 __metaclass__ = type
306
307-import unittest
308-
309+import transaction
310 from zope.component import getUtility
311
312 from canonical.launchpad.webapp.servers import LaunchpadTestRequest
313@@ -40,6 +39,7 @@
314 # translator_list composes dicts using _makeTranslatorDict.
315 group = self._makeGroup()
316 tr_translator = self.factory.makeTranslator('tr', group)
317+ transaction.commit()
318 view = self._makeView(group)
319 translator_dict = view._makeTranslatorDict(
320 tr_translator, tr_translator.language, tr_translator.translator)
321@@ -62,7 +62,3 @@
322 self.assertEqual(xhosa.datecreated, output['datecreated'])
323 self.assertEqual(xhosa.style_guide_url, output['style_guide_url'])
324 self.assertEqual(xhosa, output['context'])
325-
326-
327-def test_suite():
328- return unittest.TestLoader().loadTestsFromName(__name__)
329
330=== modified file 'lib/lp/translations/browser/translationgroup.py'
331--- lib/lp/translations/browser/translationgroup.py 2010-08-20 20:31:18 +0000
332+++ lib/lp/translations/browser/translationgroup.py 2010-09-03 11:18:46 +0000
333@@ -1,4 +1,4 @@
334-# Copyright 2009 Canonical Ltd. This software is licensed under the
335+# Copyright 2009-2010 Canonical Ltd. This software is licensed under the
336 # GNU Affero General Public License version 3 (see the file LICENSE).
337
338 """Browser code for translation groups."""
339@@ -26,6 +26,7 @@
340 LaunchpadFormView,
341 LaunchpadView,
342 )
343+from canonical.launchpad.webapp.authorization import check_permission
344 from canonical.launchpad.webapp.breadcrumb import Breadcrumb
345 from lp.app.errors import NotFoundError
346 from lp.registry.browser.objectreassignment import ObjectReassignmentView
347@@ -67,6 +68,7 @@
348 self.context = context
349 self.request = request
350 self.translation_groups = getUtility(ITranslationGroupSet)
351+ self.user_can_edit = check_permission('launchpad.Edit', self.context)
352
353 @property
354 def label(self):
355@@ -96,8 +98,7 @@
356 """List of dicts describing the translation teams."""
357 return [
358 self._makeTranslatorDict(*data)
359- for data in self.context.fetchTranslatorData()
360- ]
361+ for data in self.context.fetchTranslatorData()]
362
363
364 class TranslationGroupAddTranslatorView(LaunchpadFormView):
365
366=== modified file 'lib/lp/translations/doc/translationgroup.txt'
367--- lib/lp/translations/doc/translationgroup.txt 2010-02-22 09:06:38 +0000
368+++ lib/lp/translations/doc/translationgroup.txt 2010-09-03 11:18:46 +0000
369@@ -1,4 +1,5 @@
370-= Translation Groups =
371+Translation Groups
372+==================
373
374 >>> from zope.component import getUtility
375
376@@ -63,12 +64,12 @@
377
378 No Privileges Person isn't allowed to translate into Welsh.
379
380- >>> series = evolution.getSeries('trunk')
381- >>> subset = potemplate_set.getSubset(productseries=series)
382- >>> potemplate = subset['evolution-2.2']
383- >>> pofile = potemplate.newPOFile('cy')
384- >>> pofile.canEditTranslations(no_priv)
385- False
386+ >>> series = evolution.getSeries('trunk')
387+ >>> subset = potemplate_set.getSubset(productseries=series)
388+ >>> potemplate = subset['evolution-2.2']
389+ >>> pofile = potemplate.newPOFile('cy')
390+ >>> pofile.canEditTranslations(no_priv)
391+ False
392
393 Let's add him to the group.
394
395@@ -176,6 +177,7 @@
396 >>> de_translator = factory.makeTranslator('de', group, person=de_team)
397 >>> nl_translator = factory.makeTranslator('nl', group, person=nl_team)
398 >>> la_translator = factory.makeTranslator('la', group, person=la_team)
399+ >>> transaction.commit()
400
401 The method returns tuples of respectively a Translator ("translation
402 group membership entry"), its language, and the actual team or person
403
404=== modified file 'lib/lp/translations/interfaces/translationgroup.py'
405--- lib/lp/translations/interfaces/translationgroup.py 2010-08-20 20:31:18 +0000
406+++ lib/lp/translations/interfaces/translationgroup.py 2010-09-03 11:18:46 +0000
407@@ -186,8 +186,34 @@
408 def fetchTranslatorData():
409 """Fetch translators and related data.
410
411- :return: A tuple (`Translator`, `Language`, `Person`), ordered
412- by language name in English.
413+ Prefetches display-related properties.
414+
415+ :return: A result set of (`Translator`, `Language`, `Person`),
416+ ordered by language name in English.
417+ """
418+
419+ def fetchProjectsForDisplay():
420+ """Fetch `Product`s using this group, for display purposes.
421+
422+ Prefetches display-related properties.
423+
424+ :return: A result set of `Product`, ordered by display name.
425+ """
426+
427+ def fetchProjectGroupsForDisplay():
428+ """Fetch `Project`s using this group, for display purposes.
429+
430+ Prefetches display-related properties.
431+
432+ :return: A result set of `Project`, ordered by display name.
433+ """
434+
435+ def fetchDistrosForDisplay():
436+ """Fetch `Distribution`s using this group, for display purposes.
437+
438+ Prefetches display-related properties.
439+
440+ :return: A result set of `Distribution`, ordered by display name.
441 """
442
443
444
445=== modified file 'lib/lp/translations/model/translationgroup.py'
446--- lib/lp/translations/model/translationgroup.py 2010-08-20 20:31:18 +0000
447+++ lib/lp/translations/model/translationgroup.py 2010-09-03 11:18:46 +0000
448@@ -1,4 +1,4 @@
449-# Copyright 2009 Canonical Ltd. This software is licensed under the
450+# Copyright 2009-2010 Canonical Ltd. This software is licensed under the
451 # GNU Affero General Public License version 3 (see the file LICENSE).
452
453 # pylint: disable-msg=E0611,W0212
454@@ -6,7 +6,7 @@
455 __metaclass__ = type
456 __all__ = [
457 'TranslationGroup',
458- 'TranslationGroupSet'
459+ 'TranslationGroupSet',
460 ]
461
462 from sqlobject import (
463@@ -16,25 +16,32 @@
464 SQLRelatedJoin,
465 StringCol,
466 )
467-from storm.expr import Join
468+from storm.expr import (
469+ Join,
470+ LeftJoin,
471+ )
472 from storm.store import Store
473-from zope.component import getUtility
474 from zope.interface import implements
475
476 from canonical.database.constants import DEFAULT
477 from canonical.database.datetimecol import UtcDateTimeCol
478 from canonical.database.sqlbase import SQLBase
479-from canonical.launchpad.webapp.interfaces import (
480- DEFAULT_FLAVOR,
481- IStoreSelector,
482- MAIN_STORE,
483+from canonical.launchpad.database.librarian import (
484+ LibraryFileAlias,
485+ LibraryFileContent,
486 )
487+from canonical.launchpad.interfaces.lpstorm import ISlaveStore
488 from lp.app.errors import NotFoundError
489 from lp.registry.interfaces.person import validate_public_person
490+from lp.registry.model.distribution import Distribution
491 from lp.registry.model.person import Person
492-from lp.registry.model.product import Product
493+from lp.registry.model.product import (
494+ Product,
495+ ProductWithLicenses,
496+ )
497 from lp.registry.model.projectgroup import ProjectGroup
498 from lp.registry.model.teammembership import TeamParticipation
499+from lp.services.database.prejoin import prejoin
500 from lp.services.worlddata.model.language import Language
501 from lp.translations.interfaces.translationgroup import (
502 ITranslationGroup,
503@@ -71,16 +78,15 @@
504
505 def __getitem__(self, language_code):
506 """See `ITranslationGroup`."""
507- store = getUtility(IStoreSelector).get(MAIN_STORE, DEFAULT_FLAVOR)
508- query = store.find(
509+ query = Store.of(self).find(
510 Translator,
511 Translator.translationgroup == self,
512 Translator.languageID == Language.id,
513 Language.code == language_code)
514-
515+
516 translator = query.one()
517 if translator is None:
518- raise NotFoundError, language_code
519+ raise NotFoundError(language_code)
520
521 return translator
522
523@@ -141,14 +147,102 @@
524 return 0
525
526 def fetchTranslatorData(self):
527- """See ITranslationGroup."""
528- store = getUtility(IStoreSelector).get(MAIN_STORE, DEFAULT_FLAVOR)
529- translator_data = store.find(
530- (Translator, Language, Person),
531+ """See `ITranslationGroup`."""
532+ # Fetch Translator, Language, and Person; but also prefetch the
533+ # icon information.
534+ using = [
535+ Translator,
536+ Language,
537+ Person,
538+ LeftJoin(LibraryFileAlias, LibraryFileAlias.id == Person.iconID),
539+ LeftJoin(
540+ LibraryFileContent,
541+ LibraryFileContent.id == LibraryFileAlias.contentID),
542+ ]
543+ tables = (
544+ Translator,
545+ Language,
546+ Person,
547+ LibraryFileAlias,
548+ LibraryFileContent,
549+ )
550+ translator_data = Store.of(self).using(*using).find(
551+ tables,
552 Translator.translationgroup == self,
553 Language.id == Translator.languageID,
554 Person.id == Translator.translatorID)
555- return translator_data.order_by(Language.englishname)
556+
557+ return prejoin(
558+ translator_data.order_by(Language.englishname),
559+ slice(0, 3))
560+
561+ def fetchProjectsForDisplay(self):
562+ """See `ITranslationGroup`."""
563+ using = [
564+ Product,
565+ LeftJoin(LibraryFileAlias, LibraryFileAlias.id == Product.iconID),
566+ LeftJoin(
567+ LibraryFileContent,
568+ LibraryFileContent.id == LibraryFileAlias.contentID),
569+ ]
570+ columns = (
571+ Product,
572+ ProductWithLicenses.composeLicensesColumn(),
573+ LibraryFileAlias,
574+ LibraryFileContent,
575+ )
576+ product_data = ISlaveStore(Product).using(*using).find(
577+ columns,
578+ Product.translationgroupID == self.id, Product.active == True)
579+ product_data = product_data.order_by(Product.displayname)
580+
581+ return [
582+ ProductWithLicenses(product, tuple(licenses))
583+ for product, licenses, icon_alias, icon_content in product_data]
584+
585+ def fetchProjectGroupsForDisplay(self):
586+ """See `ITranslationGroup`."""
587+ using = [
588+ ProjectGroup,
589+ LeftJoin(
590+ LibraryFileAlias, LibraryFileAlias.id == ProjectGroup.iconID),
591+ LeftJoin(
592+ LibraryFileContent,
593+ LibraryFileContent.id == LibraryFileAlias.contentID),
594+ ]
595+ tables = (
596+ ProjectGroup,
597+ LibraryFileAlias,
598+ LibraryFileContent,
599+ )
600+ project_data = ISlaveStore(ProjectGroup).using(*using).find(
601+ tables,
602+ ProjectGroup.translationgroupID == self.id,
603+ ProjectGroup.active == True)
604+
605+ return prejoin(
606+ project_data.order_by(ProjectGroup.displayname), slice(0, 1))
607+
608+ def fetchDistrosForDisplay(self):
609+ """See `ITranslationGroup`."""
610+ using = [
611+ Distribution,
612+ LeftJoin(
613+ LibraryFileAlias, LibraryFileAlias.id == Distribution.iconID),
614+ LeftJoin(
615+ LibraryFileContent,
616+ LibraryFileContent.id == LibraryFileAlias.contentID),
617+ ]
618+ tables = (
619+ Distribution,
620+ LibraryFileAlias,
621+ LibraryFileContent,
622+ )
623+ distro_data = ISlaveStore(Distribution).using(*using).find(
624+ tables, Distribution.translationgroupID == self.id)
625+
626+ return prejoin(
627+ distro_data.order_by(Distribution.displayname), slice(0, 1))
628
629
630 class TranslationGroupSet:
631@@ -174,7 +268,7 @@
632 try:
633 return TranslationGroup.byName(name)
634 except SQLObjectNotFound:
635- raise NotFoundError, name
636+ raise NotFoundError(name)
637
638 def new(self, name, title, summary, translation_guide_url, owner):
639 """See ITranslationGroupSet."""
640@@ -193,7 +287,7 @@
641 Join(Translator,
642 Translator.translationgroupID == TranslationGroup.id),
643 Join(TeamParticipation,
644- TeamParticipation.teamID == Translator.translatorID)
645+ TeamParticipation.teamID == Translator.translatorID),
646 ]
647 result = store.using(*origin).find(
648 TranslationGroup, TeamParticipation.person == person)
649@@ -203,4 +297,3 @@
650 def getGroupsCount(self):
651 """See ITranslationGroupSet."""
652 return TranslationGroup.select().count()
653-
654
655=== modified file 'lib/lp/translations/templates/translationgroup-index.pt'
656--- lib/lp/translations/templates/translationgroup-index.pt 2010-02-22 09:06:38 +0000
657+++ lib/lp/translations/templates/translationgroup-index.pt 2010-09-03 11:18:46 +0000
658@@ -13,7 +13,7 @@
659 <a tal:content="context/owner/fmt:displayname"
660 tal:attributes="href context/owner/fmt:url"
661 class="link" />
662- <a tal:condition="context/required:launchpad.Edit"
663+ <a tal:condition="view/user_can_edit"
664 href="+reassign" title="Change administrator"
665 class="edit sprite" id="link-reassign"></a>
666 </p>
667@@ -32,7 +32,7 @@
668 translations in this translation group.</em>
669 </p>
670
671- <div tal:condition="context/required:launchpad.Edit">
672+ <div tal:condition="view/user_can_edit">
673 <a href="+edit" class="edit sprite">Change details</a>
674 </div>
675
676@@ -40,7 +40,8 @@
677
678 <div id="translation-teams-listing">
679 <h2><a name="teams"></a>Translation teams</h2>
680- <tal:translators condition="context/translators">
681+ <tal:translator-list define="translator_list view/translator_list">
682+ <tal:translators condition="translator_list">
683 <table class="sortable listing" id="group-members">
684 <thead>
685 <tr>
686@@ -48,11 +49,11 @@
687 <th>Team/Supervisor</th>
688 <th>Guidelines</th>
689 <th>Appointed</th>
690- <th tal:condition="context/required:launchpad.Edit"></th>
691+ <th tal:condition="view/user_can_edit"></th>
692 </tr>
693 </thead>
694 <tbody>
695- <tr tal:repeat="translator view/translator_list">
696+ <tr tal:repeat="translator translator_list">
697 <td class="translator-language">
698 <a tal:attributes="href translator/language/fmt:url"
699 tal:content="translator/language/displayname">
700@@ -82,7 +83,7 @@
701 none
702 </span>
703 <tal:notadmin
704- condition="not:context/required:launchpad.Edit">
705+ condition="not:view/user_can_edit">
706 &nbsp;<a tal:condition="
707 translator/context/required:launchpad.Edit"
708 tal:attributes="href string:${translator/code}/+edit"
709@@ -96,7 +97,7 @@
710 2007-09-17
711 </span>
712 </td>
713- <td tal:condition="context/required:launchpad.Edit">
714+ <td tal:condition="view/user_can_edit">
715 <a tal:attributes="
716 href translator/code;
717 id string:edit-${translator/code}-translator"
718@@ -110,22 +111,22 @@
719 </tbody>
720 </table>
721 </tal:translators>
722- <tal:no-translators condition="not: context/translators">
723+ <tal:no-translators condition="not: translator_list">
724 No translation teams or supervisors have been appointed in
725 this group yet.
726 </tal:no-translators>
727 <div style="margin-top:1em; margin-bottom: 2em;">
728- <a tal:condition="context/required:launchpad.Edit"
729+ <a tal:condition="view/user_can_edit"
730 href="+appoint" class="add sprite">Appoint a new translation
731 team</a>
732 </div>
733- </div><!-- id="translations-team-listing" -->
734-
735- <div class="section">
736- <a name="projects"></a>
737- <div tal:replace="structure context/@@+portlet-projects" />
738- </div>
739-
740+ </tal:translator-list>
741+ </div><!-- id="translations-team-listing" -->
742+
743+ <div class="section">
744+ <a name="projects"></a>
745+ <div tal:replace="structure context/@@+portlet-projects" />
746+ </div>
747 </div><!-- main -->
748
749 </body>
750
751=== modified file 'lib/lp/translations/templates/translationgroup-portlet-projects.pt'
752--- lib/lp/translations/templates/translationgroup-portlet-projects.pt 2010-04-19 08:11:52 +0000
753+++ lib/lp/translations/templates/translationgroup-portlet-projects.pt 2010-09-03 11:18:46 +0000
754@@ -13,33 +13,34 @@
755 <a href="#teams">translation teams</a>.
756 </p>
757
758- <div id="related-projects">
759- <div tal:condition="context/distributions" style="margin-top:1em;">
760+ <div id="related-projects"
761+ tal:define="
762+ distributions context/fetchDistrosForDisplay;
763+ projectgroups context/fetchProjectGroupsForDisplay;
764+ projects context/fetchProjectsForDisplay">
765+ <div tal:condition="distributions" style="margin-top:1em;">
766 <h3 style="display: inline;">Distributions:</h3>
767- <tal:distribution
768- repeat="distribution context/distributions">
769+ <tal:distribution repeat="distribution distributions">
770 <a href="#" tal:replace="structure distribution/fmt:link">Ubuntu
771 </a><tal:comma condition="not:repeat/distribution/end">, </tal:comma>
772 </tal:distribution>
773 </div>
774
775- <div tal:condition="context/projects" style="margin-top:1em;">
776+ <div tal:condition="projectgroups" style="margin-top:1em;">
777 <h3 style="display: inline;">Project groups:</h3>
778- <tal:project
779- repeat="projectgroup context/projects">
780+ <tal:projectgroup repeat="projectgroup projectgroups">
781 <a href="#" tal:replace="structure projectgroup/fmt:link">GNOME
782 </a><tal:comma condition="not:repeat/projectgroup/end">, </tal:comma>
783+ </tal:projectgroup>
784+ </div>
785+
786+ <div tal:condition="projects" style="margin-top:1em;">
787+ <h3 style="display: inline;">Projects:</h3>
788+ <tal:project repeat="project projects">
789+ <a href="#" tal:replace="structure project/fmt:link">Firefox
790+ </a><tal:comma condition="not:repeat/project/end">, </tal:comma>
791 </tal:project>
792 </div>
793
794- <div tal:condition="context/products" style="margin-top:1em;">
795- <h3 style="display: inline;">Projects:</h3>
796- <tal:product
797- repeat="product context/products">
798- <a href="#" tal:replace="structure product/fmt:link">Firefox
799- </a><tal:comma condition="not:repeat/product/end">, </tal:comma>
800- </tal:product>
801- </div>
802-
803 </div>
804 </tal:root>