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
=== modified file 'lib/lp/registry/model/pillar.py'
--- lib/lp/registry/model/pillar.py 2010-08-20 20:31:18 +0000
+++ lib/lp/registry/model/pillar.py 2010-09-03 11:18:46 +0000
@@ -17,11 +17,7 @@
17 ForeignKey,17 ForeignKey,
18 StringCol,18 StringCol,
19 )19 )
20from storm.expr import (20from storm.expr import LeftJoin
21 LeftJoin,
22 NamedFunc,
23 Select,
24 )
25from storm.info import ClassAlias21from storm.info import ClassAlias
26from storm.locals import SQL22from storm.locals import SQL
27from storm.store import Store23from storm.store import Store
@@ -51,11 +47,9 @@
51from lp.registry.interfaces.product import (47from lp.registry.interfaces.product import (
52 IProduct,48 IProduct,
53 IProductSet,49 IProductSet,
54 License,
55 )50 )
56from lp.registry.interfaces.projectgroup import IProjectGroupSet51from lp.registry.interfaces.projectgroup import IProjectGroupSet
57from lp.registry.model.featuredproject import FeaturedProject52from lp.registry.model.featuredproject import FeaturedProject
58from lp.registry.model.productlicense import ProductLicense
5953
6054
61__all__ = [55__all__ = [
@@ -197,23 +191,15 @@
197191
198 def search(self, text, limit):192 def search(self, text, limit):
199 """See `IPillarSet`."""193 """See `IPillarSet`."""
200 # These classes are imported in this method to prevent an import loop.194 # Avoid circular import.
201 from lp.registry.model.product import (195 from lp.registry.model.product import ProductWithLicenses
202 Product, ProductWithLicenses)
203 if limit is None:196 if limit is None:
204 limit = config.launchpad.default_batch_size197 limit = config.launchpad.default_batch_size
205198
206 class Array(NamedFunc):
207 """Storm representation of the array() PostgreSQL function."""
208 name = 'array'
209
210 # Pull out the licenses as a subselect which is converted199 # Pull out the licenses as a subselect which is converted
211 # into a PostgreSQL array so that multiple licenses per product200 # into a PostgreSQL array so that multiple licenses per product
212 # can be retrieved in a single row for each product.201 # can be retrieved in a single row for each product.
213 extra_column = Array(202 extra_column = ProductWithLicenses.composeLicensesColumn()
214 Select(columns=[ProductLicense.license],
215 where=(ProductLicense.product == Product.id),
216 tables=[ProductLicense]))
217 result = self.build_search_query(text, [extra_column])203 result = self.build_search_query(text, [extra_column])
218204
219 # If the search text matches the name or title of the205 # If the search text matches the name or title of the
@@ -244,18 +230,12 @@
244 stacklevel=2)230 stacklevel=2)
245 pillars = []231 pillars = []
246 # Prefill pillar.product.licenses.232 # Prefill pillar.product.licenses.
247 for pillar_name, other, product, project, distro, license_ids in (233 for pillar_name, other, product, project, distro, licenses in (
248 result[:limit]):234 result[:limit]):
249 pillar = pillar_name.pillar235 pillar = pillar_name.pillar
250 if IProduct.providedBy(pillar):236 if IProduct.providedBy(pillar):
251 licenses = [237 pillar = ProductWithLicenses(pillar, licenses)
252 License.items[license_id]238 pillars.append(pillar)
253 for license_id in license_ids]
254 product_with_licenses = ProductWithLicenses(
255 pillar, tuple(sorted(licenses)))
256 pillars.append(product_with_licenses)
257 else:
258 pillars.append(pillar)
259 return pillars239 return pillars
260240
261 def add_featured_project(self, project):241 def add_featured_project(self, project):
262242
=== modified file 'lib/lp/registry/model/product.py'
--- lib/lp/registry/model/product.py 2010-09-03 03:12:39 +0000
+++ lib/lp/registry/model/product.py 2010-09-03 11:18:46 +0000
@@ -1,4 +1,4 @@
1# Copyright 2009 Canonical Ltd. This software is licensed under the1# Copyright 2009-2010 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).2# GNU Affero General Public License version 3 (see the file LICENSE).
3# pylint: disable-msg=E0611,W02123# pylint: disable-msg=E0611,W0212
44
@@ -25,11 +25,13 @@
25 SQLObjectNotFound,25 SQLObjectNotFound,
26 StringCol,26 StringCol,
27 )27 )
28from storm.expr import NamedFunc
28from storm.locals import (29from storm.locals import (
29 And,30 And,
30 Desc,31 Desc,
31 Int,32 Int,
32 Join,33 Join,
34 Select,
33 SQL,35 SQL,
34 Store,36 Store,
35 Unicode,37 Unicode,
@@ -85,7 +87,6 @@
85 SpecificationDefinitionStatus,87 SpecificationDefinitionStatus,
86 SpecificationFilter,88 SpecificationFilter,
87 SpecificationImplementationStatus,89 SpecificationImplementationStatus,
88 SpecificationSort,
89 )90 )
90from lp.blueprints.model.specification import (91from lp.blueprints.model.specification import (
91 HasSpecificationsMixin,92 HasSpecificationsMixin,
@@ -201,14 +202,25 @@
201 return LicenseStatus.OPEN_SOURCE202 return LicenseStatus.OPEN_SOURCE
202203
203204
205class Array(NamedFunc):
206 """Implements the postgres "array" function in Storm."""
207 name = 'array'
208
209
204class ProductWithLicenses:210class ProductWithLicenses:
205 """Caches `Product.licenses`."""211 """Caches `Product.licenses`."""
206212
207 delegates(IProduct, 'product')213 delegates(IProduct, 'product')
208214
209 def __init__(self, product, licenses):215 def __init__(self, product, license_ids):
216 """Initialize a `ProductWithLicenses`.
217
218 :param product: the `Product` to wrap.
219 :param license_ids: a sequence of numeric `License` ids.
220 """
210 self.product = product221 self.product = product
211 self._licenses = licenses222 self._licenses = tuple([
223 License.items[id] for id in sorted(license_ids)])
212224
213 @property225 @property
214 def licenses(self):226 def licenses(self):
@@ -223,8 +235,40 @@
223 `Product.licenses`, which is not cached, instead of235 `Product.licenses`, which is not cached, instead of
224 `ProductWithLicenses.licenses`, which is cached.236 `ProductWithLicenses.licenses`, which is cached.
225 """237 """
238 naked_product = removeSecurityProxy(self.product)
226 return get_license_status(239 return get_license_status(
227 self.license_approved, self.license_reviewed, self.licenses)240 naked_product.license_approved, naked_product.license_reviewed,
241 self.licenses)
242
243 @classmethod
244 def composeLicensesColumn(cls, for_class=None):
245 """Compose a Storm column specification for licenses.
246
247 Use this to render a list of `Product` linkes without querying
248 licenses for each one individually.
249
250 It lets you prefetch the licensing information in the same
251 query that fetches a `Product`. Just add the column spec
252 returned by this function to the query, and pass it to the
253 `ProductWithLicenses` constructor:
254
255 license_column = ProductWithLicenses.composeLicensesColumn()
256 products_with_licenses = [
257 ProductWithLicenses(product, licenses)
258 for product, licenses in store.find(Product, license_column)
259 ]
260
261 :param for_class: Class to find licenses for. Defaults to
262 `Product`, but could also be a Storm `ClassAlias`.
263 """
264 if for_class is None:
265 for_class = Product
266
267 return Array(
268 Select(
269 columns=[ProductLicense.license],
270 where=(ProductLicense.product == for_class.id),
271 tables=[ProductLicense]))
228272
229273
230class Product(SQLBase, BugTargetBase, MakesAnnouncements,274class Product(SQLBase, BugTargetBase, MakesAnnouncements,
231275
=== added file 'lib/lp/registry/tests/test_productwithlicenses.py'
--- lib/lp/registry/tests/test_productwithlicenses.py 1970-01-01 00:00:00 +0000
+++ lib/lp/registry/tests/test_productwithlicenses.py 2010-09-03 11:18:46 +0000
@@ -0,0 +1,114 @@
1# Copyright 2010 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).
3
4"""Unit tests for `ProductWithLicenses`."""
5
6__metaclass__ = type
7
8from operator import attrgetter
9
10from storm.store import Store
11from zope.interface.verify import verifyObject
12
13from canonical.launchpad.ftests import (
14 ANONYMOUS,
15 login,
16 )
17from canonical.testing import DatabaseFunctionalLayer
18from lp.registry.interfaces.product import (
19 IProduct,
20 License,
21 LicenseStatus,
22 )
23from lp.registry.model.product import (
24 Product,
25 ProductWithLicenses,
26 )
27from lp.testing import TestCaseWithFactory
28
29
30class TestProductWithLicenses(TestCaseWithFactory):
31
32 layer = DatabaseFunctionalLayer
33
34 def test_baseline(self):
35 product = self.factory.makeProduct()
36 product_with_licenses = ProductWithLicenses(product, [])
37 # Log in--a full verification takes Edit privileges.
38 login('foo.bar@canonical.com')
39 self.assertTrue(verifyObject(IProduct, product_with_licenses))
40
41 def test_uses_cached_licenses(self):
42 # The ProductWithLicenses' licensing information is based purely
43 # on the cached licenses list. The database is not queried to
44 # determine licensing status.
45 product = self.factory.makeProduct(licenses=[License.BSD])
46 product_with_licenses = ProductWithLicenses(
47 product, [License.OTHER_PROPRIETARY.value])
48 license_status = product_with_licenses.license_status
49 self.assertEqual(LicenseStatus.PROPRIETARY, license_status)
50
51 def test_sorts_licenses(self):
52 # The ProductWithLicenses constructor sorts the Licenses by
53 # numeric value.
54 product = self.factory.makeProduct()
55 licenses = [License.AFFERO, License.BSD, License.MIT]
56
57 # Feed the constructor a list of ids in the wrong order.
58 product_with_licenses = ProductWithLicenses(
59 product,
60 sorted([license.value for license in licenses], reverse=True))
61
62 expected = sorted(licenses, key=attrgetter('value'))
63 self.assertEqual(tuple(expected), product_with_licenses.licenses)
64
65 def test_compose_column_without_licenses_produces_empty(self):
66 # The licenses column that ProductWithLicenses produces for a
67 # product without licenses contains an empty list.
68 product = self.factory.makeProduct(licenses=[])
69 column = ProductWithLicenses.composeLicensesColumn()
70 store = Store.of(product)
71 result = list(store.find((Product, column), Product.id == product.id))
72
73 self.assertEqual([(product, [])], result)
74
75 def test_licenses_column_contains_licensing_info(self):
76 # Feeding the licenses column into the ProductWithLicenses
77 # constructor seeds it with the appropriate licenses.
78 product = self.factory.makeProduct(
79 licenses=[License.OTHER_PROPRIETARY])
80 column = ProductWithLicenses.composeLicensesColumn()
81 store = Store.of(product)
82 row = store.find((Product, column), Product.id == product.id).one()
83
84 product_with_licenses = ProductWithLicenses(*row)
85 licenses = product_with_licenses.licenses
86 license_status = product_with_licenses.license_status
87 self.assertEqual((License.OTHER_PROPRIETARY, ), licenses)
88 self.assertEqual(LicenseStatus.PROPRIETARY, license_status)
89
90 def test_licenses_column_aggregates(self):
91 # Adding a licensing column for a product with multiple licenses
92 # still finds a single product, not one per license.
93 licenses = [License.AFFERO, License.GNU_GPL_V3]
94 product = self.factory.makeProduct(licenses=licenses)
95 column = ProductWithLicenses.composeLicensesColumn()
96 store = Store.of(product)
97 result = list(store.find((Product, column), Product.id == product.id))
98
99 self.assertEqual(1, len(result))
100 found_product, found_licenses = result[0]
101 self.assertEqual(product, found_product)
102 self.assertEqual(len(licenses), len(found_licenses))
103
104 def test_license_status_is_public(self):
105 # The license_status attribute can be read by anyone, on
106 # ProductWithLicenses as on Product.
107 product = self.factory.makeProduct(licenses=[License.BSD])
108 product_with_licenses = ProductWithLicenses(
109 product, [License.BSD.value])
110 login(ANONYMOUS)
111 self.assertEqual(
112 LicenseStatus.OPEN_SOURCE, product.license_status)
113 self.assertEqual(
114 LicenseStatus.OPEN_SOURCE, product_with_licenses.license_status)
0115
=== modified file 'lib/lp/translations/browser/tests/test_translationgroup.py'
--- lib/lp/translations/browser/tests/test_translationgroup.py 2010-08-20 20:31:18 +0000
+++ lib/lp/translations/browser/tests/test_translationgroup.py 2010-09-03 11:18:46 +0000
@@ -5,8 +5,7 @@
55
6__metaclass__ = type6__metaclass__ = type
77
8import unittest8import transaction
9
10from zope.component import getUtility9from zope.component import getUtility
1110
12from canonical.launchpad.webapp.servers import LaunchpadTestRequest11from canonical.launchpad.webapp.servers import LaunchpadTestRequest
@@ -40,6 +39,7 @@
40 # translator_list composes dicts using _makeTranslatorDict.39 # translator_list composes dicts using _makeTranslatorDict.
41 group = self._makeGroup()40 group = self._makeGroup()
42 tr_translator = self.factory.makeTranslator('tr', group)41 tr_translator = self.factory.makeTranslator('tr', group)
42 transaction.commit()
43 view = self._makeView(group)43 view = self._makeView(group)
44 translator_dict = view._makeTranslatorDict(44 translator_dict = view._makeTranslatorDict(
45 tr_translator, tr_translator.language, tr_translator.translator)45 tr_translator, tr_translator.language, tr_translator.translator)
@@ -62,7 +62,3 @@
62 self.assertEqual(xhosa.datecreated, output['datecreated'])62 self.assertEqual(xhosa.datecreated, output['datecreated'])
63 self.assertEqual(xhosa.style_guide_url, output['style_guide_url'])63 self.assertEqual(xhosa.style_guide_url, output['style_guide_url'])
64 self.assertEqual(xhosa, output['context'])64 self.assertEqual(xhosa, output['context'])
65
66
67def test_suite():
68 return unittest.TestLoader().loadTestsFromName(__name__)
6965
=== modified file 'lib/lp/translations/browser/translationgroup.py'
--- lib/lp/translations/browser/translationgroup.py 2010-08-20 20:31:18 +0000
+++ lib/lp/translations/browser/translationgroup.py 2010-09-03 11:18:46 +0000
@@ -1,4 +1,4 @@
1# Copyright 2009 Canonical Ltd. This software is licensed under the1# Copyright 2009-2010 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).2# GNU Affero General Public License version 3 (see the file LICENSE).
33
4"""Browser code for translation groups."""4"""Browser code for translation groups."""
@@ -26,6 +26,7 @@
26 LaunchpadFormView,26 LaunchpadFormView,
27 LaunchpadView,27 LaunchpadView,
28 )28 )
29from canonical.launchpad.webapp.authorization import check_permission
29from canonical.launchpad.webapp.breadcrumb import Breadcrumb30from canonical.launchpad.webapp.breadcrumb import Breadcrumb
30from lp.app.errors import NotFoundError31from lp.app.errors import NotFoundError
31from lp.registry.browser.objectreassignment import ObjectReassignmentView32from lp.registry.browser.objectreassignment import ObjectReassignmentView
@@ -67,6 +68,7 @@
67 self.context = context68 self.context = context
68 self.request = request69 self.request = request
69 self.translation_groups = getUtility(ITranslationGroupSet)70 self.translation_groups = getUtility(ITranslationGroupSet)
71 self.user_can_edit = check_permission('launchpad.Edit', self.context)
7072
71 @property73 @property
72 def label(self):74 def label(self):
@@ -96,8 +98,7 @@
96 """List of dicts describing the translation teams."""98 """List of dicts describing the translation teams."""
97 return [99 return [
98 self._makeTranslatorDict(*data)100 self._makeTranslatorDict(*data)
99 for data in self.context.fetchTranslatorData()101 for data in self.context.fetchTranslatorData()]
100 ]
101102
102103
103class TranslationGroupAddTranslatorView(LaunchpadFormView):104class TranslationGroupAddTranslatorView(LaunchpadFormView):
104105
=== modified file 'lib/lp/translations/doc/translationgroup.txt'
--- lib/lp/translations/doc/translationgroup.txt 2010-02-22 09:06:38 +0000
+++ lib/lp/translations/doc/translationgroup.txt 2010-09-03 11:18:46 +0000
@@ -1,4 +1,5 @@
1= Translation Groups =1Translation Groups
2==================
23
3 >>> from zope.component import getUtility4 >>> from zope.component import getUtility
45
@@ -63,12 +64,12 @@
6364
64No Privileges Person isn't allowed to translate into Welsh.65No Privileges Person isn't allowed to translate into Welsh.
6566
66 >>> series = evolution.getSeries('trunk')67 >>> series = evolution.getSeries('trunk')
67 >>> subset = potemplate_set.getSubset(productseries=series)68 >>> subset = potemplate_set.getSubset(productseries=series)
68 >>> potemplate = subset['evolution-2.2']69 >>> potemplate = subset['evolution-2.2']
69 >>> pofile = potemplate.newPOFile('cy')70 >>> pofile = potemplate.newPOFile('cy')
70 >>> pofile.canEditTranslations(no_priv)71 >>> pofile.canEditTranslations(no_priv)
71 False72 False
7273
73Let's add him to the group.74Let's add him to the group.
7475
@@ -176,6 +177,7 @@
176 >>> de_translator = factory.makeTranslator('de', group, person=de_team)177 >>> de_translator = factory.makeTranslator('de', group, person=de_team)
177 >>> nl_translator = factory.makeTranslator('nl', group, person=nl_team)178 >>> nl_translator = factory.makeTranslator('nl', group, person=nl_team)
178 >>> la_translator = factory.makeTranslator('la', group, person=la_team)179 >>> la_translator = factory.makeTranslator('la', group, person=la_team)
180 >>> transaction.commit()
179181
180The method returns tuples of respectively a Translator ("translation182The method returns tuples of respectively a Translator ("translation
181group membership entry"), its language, and the actual team or person183group membership entry"), its language, and the actual team or person
182184
=== 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 11:18:46 +0000
@@ -186,8 +186,34 @@
186 def fetchTranslatorData():186 def fetchTranslatorData():
187 """Fetch translators and related data.187 """Fetch translators and related data.
188188
189 :return: A tuple (`Translator`, `Language`, `Person`), ordered189 Prefetches display-related properties.
190 by language name in English.190
191 :return: A result set of (`Translator`, `Language`, `Person`),
192 ordered by language name in English.
193 """
194
195 def fetchProjectsForDisplay():
196 """Fetch `Product`s using this group, for display purposes.
197
198 Prefetches display-related properties.
199
200 :return: A result set of `Product`, ordered by display name.
201 """
202
203 def fetchProjectGroupsForDisplay():
204 """Fetch `Project`s using this group, for display purposes.
205
206 Prefetches display-related properties.
207
208 :return: A result set of `Project`, ordered by display name.
209 """
210
211 def fetchDistrosForDisplay():
212 """Fetch `Distribution`s using this group, for display purposes.
213
214 Prefetches display-related properties.
215
216 :return: A result set of `Distribution`, ordered by display name.
191 """217 """
192218
193219
194220
=== modified file 'lib/lp/translations/model/translationgroup.py'
--- lib/lp/translations/model/translationgroup.py 2010-08-20 20:31:18 +0000
+++ lib/lp/translations/model/translationgroup.py 2010-09-03 11:18:46 +0000
@@ -1,4 +1,4 @@
1# Copyright 2009 Canonical Ltd. This software is licensed under the1# Copyright 2009-2010 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).2# GNU Affero General Public License version 3 (see the file LICENSE).
33
4# pylint: disable-msg=E0611,W02124# pylint: disable-msg=E0611,W0212
@@ -6,7 +6,7 @@
6__metaclass__ = type6__metaclass__ = type
7__all__ = [7__all__ = [
8 'TranslationGroup',8 'TranslationGroup',
9 'TranslationGroupSet'9 'TranslationGroupSet',
10 ]10 ]
1111
12from sqlobject import (12from sqlobject import (
@@ -16,25 +16,32 @@
16 SQLRelatedJoin,16 SQLRelatedJoin,
17 StringCol,17 StringCol,
18 )18 )
19from storm.expr import Join19from storm.expr import (
20 Join,
21 LeftJoin,
22 )
20from storm.store import Store23from storm.store import Store
21from zope.component import getUtility
22from zope.interface import implements24from zope.interface import implements
2325
24from canonical.database.constants import DEFAULT26from canonical.database.constants import DEFAULT
25from canonical.database.datetimecol import UtcDateTimeCol27from canonical.database.datetimecol import UtcDateTimeCol
26from canonical.database.sqlbase import SQLBase28from canonical.database.sqlbase import SQLBase
27from canonical.launchpad.webapp.interfaces import (29from canonical.launchpad.database.librarian import (
28 DEFAULT_FLAVOR,30 LibraryFileAlias,
29 IStoreSelector,31 LibraryFileContent,
30 MAIN_STORE,
31 )32 )
33from canonical.launchpad.interfaces.lpstorm import ISlaveStore
32from lp.app.errors import NotFoundError34from lp.app.errors import NotFoundError
33from lp.registry.interfaces.person import validate_public_person35from lp.registry.interfaces.person import validate_public_person
36from lp.registry.model.distribution import Distribution
34from lp.registry.model.person import Person37from lp.registry.model.person import Person
35from lp.registry.model.product import Product38from lp.registry.model.product import (
39 Product,
40 ProductWithLicenses,
41 )
36from lp.registry.model.projectgroup import ProjectGroup42from lp.registry.model.projectgroup import ProjectGroup
37from lp.registry.model.teammembership import TeamParticipation43from lp.registry.model.teammembership import TeamParticipation
44from lp.services.database.prejoin import prejoin
38from lp.services.worlddata.model.language import Language45from lp.services.worlddata.model.language import Language
39from lp.translations.interfaces.translationgroup import (46from lp.translations.interfaces.translationgroup import (
40 ITranslationGroup,47 ITranslationGroup,
@@ -71,16 +78,15 @@
7178
72 def __getitem__(self, language_code):79 def __getitem__(self, language_code):
73 """See `ITranslationGroup`."""80 """See `ITranslationGroup`."""
74 store = getUtility(IStoreSelector).get(MAIN_STORE, DEFAULT_FLAVOR)81 query = Store.of(self).find(
75 query = store.find(
76 Translator,82 Translator,
77 Translator.translationgroup == self,83 Translator.translationgroup == self,
78 Translator.languageID == Language.id,84 Translator.languageID == Language.id,
79 Language.code == language_code)85 Language.code == language_code)
80 86
81 translator = query.one()87 translator = query.one()
82 if translator is None:88 if translator is None:
83 raise NotFoundError, language_code89 raise NotFoundError(language_code)
8490
85 return translator91 return translator
8692
@@ -141,14 +147,102 @@
141 return 0147 return 0
142148
143 def fetchTranslatorData(self):149 def fetchTranslatorData(self):
144 """See ITranslationGroup."""150 """See `ITranslationGroup`."""
145 store = getUtility(IStoreSelector).get(MAIN_STORE, DEFAULT_FLAVOR)151 # Fetch Translator, Language, and Person; but also prefetch the
146 translator_data = store.find(152 # icon information.
147 (Translator, Language, Person),153 using = [
154 Translator,
155 Language,
156 Person,
157 LeftJoin(LibraryFileAlias, LibraryFileAlias.id == Person.iconID),
158 LeftJoin(
159 LibraryFileContent,
160 LibraryFileContent.id == LibraryFileAlias.contentID),
161 ]
162 tables = (
163 Translator,
164 Language,
165 Person,
166 LibraryFileAlias,
167 LibraryFileContent,
168 )
169 translator_data = Store.of(self).using(*using).find(
170 tables,
148 Translator.translationgroup == self,171 Translator.translationgroup == self,
149 Language.id == Translator.languageID,172 Language.id == Translator.languageID,
150 Person.id == Translator.translatorID)173 Person.id == Translator.translatorID)
151 return translator_data.order_by(Language.englishname)174
175 return prejoin(
176 translator_data.order_by(Language.englishname),
177 slice(0, 3))
178
179 def fetchProjectsForDisplay(self):
180 """See `ITranslationGroup`."""
181 using = [
182 Product,
183 LeftJoin(LibraryFileAlias, LibraryFileAlias.id == Product.iconID),
184 LeftJoin(
185 LibraryFileContent,
186 LibraryFileContent.id == LibraryFileAlias.contentID),
187 ]
188 columns = (
189 Product,
190 ProductWithLicenses.composeLicensesColumn(),
191 LibraryFileAlias,
192 LibraryFileContent,
193 )
194 product_data = ISlaveStore(Product).using(*using).find(
195 columns,
196 Product.translationgroupID == self.id, Product.active == True)
197 product_data = product_data.order_by(Product.displayname)
198
199 return [
200 ProductWithLicenses(product, tuple(licenses))
201 for product, licenses, icon_alias, icon_content in product_data]
202
203 def fetchProjectGroupsForDisplay(self):
204 """See `ITranslationGroup`."""
205 using = [
206 ProjectGroup,
207 LeftJoin(
208 LibraryFileAlias, LibraryFileAlias.id == ProjectGroup.iconID),
209 LeftJoin(
210 LibraryFileContent,
211 LibraryFileContent.id == LibraryFileAlias.contentID),
212 ]
213 tables = (
214 ProjectGroup,
215 LibraryFileAlias,
216 LibraryFileContent,
217 )
218 project_data = ISlaveStore(ProjectGroup).using(*using).find(
219 tables,
220 ProjectGroup.translationgroupID == self.id,
221 ProjectGroup.active == True)
222
223 return prejoin(
224 project_data.order_by(ProjectGroup.displayname), slice(0, 1))
225
226 def fetchDistrosForDisplay(self):
227 """See `ITranslationGroup`."""
228 using = [
229 Distribution,
230 LeftJoin(
231 LibraryFileAlias, LibraryFileAlias.id == Distribution.iconID),
232 LeftJoin(
233 LibraryFileContent,
234 LibraryFileContent.id == LibraryFileAlias.contentID),
235 ]
236 tables = (
237 Distribution,
238 LibraryFileAlias,
239 LibraryFileContent,
240 )
241 distro_data = ISlaveStore(Distribution).using(*using).find(
242 tables, Distribution.translationgroupID == self.id)
243
244 return prejoin(
245 distro_data.order_by(Distribution.displayname), slice(0, 1))
152246
153247
154class TranslationGroupSet:248class TranslationGroupSet:
@@ -174,7 +268,7 @@
174 try:268 try:
175 return TranslationGroup.byName(name)269 return TranslationGroup.byName(name)
176 except SQLObjectNotFound:270 except SQLObjectNotFound:
177 raise NotFoundError, name271 raise NotFoundError(name)
178272
179 def new(self, name, title, summary, translation_guide_url, owner):273 def new(self, name, title, summary, translation_guide_url, owner):
180 """See ITranslationGroupSet."""274 """See ITranslationGroupSet."""
@@ -193,7 +287,7 @@
193 Join(Translator,287 Join(Translator,
194 Translator.translationgroupID == TranslationGroup.id),288 Translator.translationgroupID == TranslationGroup.id),
195 Join(TeamParticipation,289 Join(TeamParticipation,
196 TeamParticipation.teamID == Translator.translatorID)290 TeamParticipation.teamID == Translator.translatorID),
197 ]291 ]
198 result = store.using(*origin).find(292 result = store.using(*origin).find(
199 TranslationGroup, TeamParticipation.person == person)293 TranslationGroup, TeamParticipation.person == person)
@@ -203,4 +297,3 @@
203 def getGroupsCount(self):297 def getGroupsCount(self):
204 """See ITranslationGroupSet."""298 """See ITranslationGroupSet."""
205 return TranslationGroup.select().count()299 return TranslationGroup.select().count()
206
207300
=== modified file 'lib/lp/translations/templates/translationgroup-index.pt'
--- lib/lp/translations/templates/translationgroup-index.pt 2010-02-22 09:06:38 +0000
+++ lib/lp/translations/templates/translationgroup-index.pt 2010-09-03 11:18:46 +0000
@@ -13,7 +13,7 @@
13 <a tal:content="context/owner/fmt:displayname"13 <a tal:content="context/owner/fmt:displayname"
14 tal:attributes="href context/owner/fmt:url"14 tal:attributes="href context/owner/fmt:url"
15 class="link" />15 class="link" />
16 <a tal:condition="context/required:launchpad.Edit"16 <a tal:condition="view/user_can_edit"
17 href="+reassign" title="Change administrator"17 href="+reassign" title="Change administrator"
18 class="edit sprite" id="link-reassign"></a>18 class="edit sprite" id="link-reassign"></a>
19 </p>19 </p>
@@ -32,7 +32,7 @@
32 translations in this translation group.</em>32 translations in this translation group.</em>
33 </p>33 </p>
3434
35 <div tal:condition="context/required:launchpad.Edit">35 <div tal:condition="view/user_can_edit">
36 <a href="+edit" class="edit sprite">Change details</a>36 <a href="+edit" class="edit sprite">Change details</a>
37 </div>37 </div>
3838
@@ -40,7 +40,8 @@
4040
41 <div id="translation-teams-listing">41 <div id="translation-teams-listing">
42 <h2><a name="teams"></a>Translation teams</h2>42 <h2><a name="teams"></a>Translation teams</h2>
43 <tal:translators condition="context/translators">43 <tal:translator-list define="translator_list view/translator_list">
44 <tal:translators condition="translator_list">
44 <table class="sortable listing" id="group-members">45 <table class="sortable listing" id="group-members">
45 <thead>46 <thead>
46 <tr>47 <tr>
@@ -48,11 +49,11 @@
48 <th>Team/Supervisor</th>49 <th>Team/Supervisor</th>
49 <th>Guidelines</th>50 <th>Guidelines</th>
50 <th>Appointed</th>51 <th>Appointed</th>
51 <th tal:condition="context/required:launchpad.Edit"></th>52 <th tal:condition="view/user_can_edit"></th>
52 </tr>53 </tr>
53 </thead>54 </thead>
54 <tbody>55 <tbody>
55 <tr tal:repeat="translator view/translator_list">56 <tr tal:repeat="translator translator_list">
56 <td class="translator-language">57 <td class="translator-language">
57 <a tal:attributes="href translator/language/fmt:url"58 <a tal:attributes="href translator/language/fmt:url"
58 tal:content="translator/language/displayname">59 tal:content="translator/language/displayname">
@@ -82,7 +83,7 @@
82 none83 none
83 </span>84 </span>
84 <tal:notadmin85 <tal:notadmin
85 condition="not:context/required:launchpad.Edit">86 condition="not:view/user_can_edit">
86 &nbsp;<a tal:condition="87 &nbsp;<a tal:condition="
87 translator/context/required:launchpad.Edit"88 translator/context/required:launchpad.Edit"
88 tal:attributes="href string:${translator/code}/+edit"89 tal:attributes="href string:${translator/code}/+edit"
@@ -96,7 +97,7 @@
96 2007-09-1797 2007-09-17
97 </span>98 </span>
98 </td>99 </td>
99 <td tal:condition="context/required:launchpad.Edit">100 <td tal:condition="view/user_can_edit">
100 <a tal:attributes="101 <a tal:attributes="
101 href translator/code;102 href translator/code;
102 id string:edit-${translator/code}-translator"103 id string:edit-${translator/code}-translator"
@@ -110,22 +111,22 @@
110 </tbody>111 </tbody>
111 </table>112 </table>
112 </tal:translators>113 </tal:translators>
113 <tal:no-translators condition="not: context/translators">114 <tal:no-translators condition="not: translator_list">
114 No translation teams or supervisors have been appointed in115 No translation teams or supervisors have been appointed in
115 this group yet.116 this group yet.
116 </tal:no-translators>117 </tal:no-translators>
117 <div style="margin-top:1em; margin-bottom: 2em;">118 <div style="margin-top:1em; margin-bottom: 2em;">
118 <a tal:condition="context/required:launchpad.Edit"119 <a tal:condition="view/user_can_edit"
119 href="+appoint" class="add sprite">Appoint a new translation120 href="+appoint" class="add sprite">Appoint a new translation
120 team</a>121 team</a>
121 </div>122 </div>
122 </div><!-- id="translations-team-listing" -->123 </tal:translator-list>
123124 </div><!-- id="translations-team-listing" -->
124 <div class="section">125
125 <a name="projects"></a>126 <div class="section">
126 <div tal:replace="structure context/@@+portlet-projects" />127 <a name="projects"></a>
127 </div>128 <div tal:replace="structure context/@@+portlet-projects" />
128129 </div>
129 </div><!-- main -->130 </div><!-- main -->
130131
131 </body>132 </body>
132133
=== modified file 'lib/lp/translations/templates/translationgroup-portlet-projects.pt'
--- lib/lp/translations/templates/translationgroup-portlet-projects.pt 2010-04-19 08:11:52 +0000
+++ lib/lp/translations/templates/translationgroup-portlet-projects.pt 2010-09-03 11:18:46 +0000
@@ -13,33 +13,34 @@
13 <a href="#teams">translation teams</a>.13 <a href="#teams">translation teams</a>.
14 </p>14 </p>
1515
16 <div id="related-projects">16 <div id="related-projects"
17 <div tal:condition="context/distributions" style="margin-top:1em;">17 tal:define="
18 distributions context/fetchDistrosForDisplay;
19 projectgroups context/fetchProjectGroupsForDisplay;
20 projects context/fetchProjectsForDisplay">
21 <div tal:condition="distributions" style="margin-top:1em;">
18 <h3 style="display: inline;">Distributions:</h3>22 <h3 style="display: inline;">Distributions:</h3>
19 <tal:distribution23 <tal:distribution repeat="distribution distributions">
20 repeat="distribution context/distributions">
21 <a href="#" tal:replace="structure distribution/fmt:link">Ubuntu24 <a href="#" tal:replace="structure distribution/fmt:link">Ubuntu
22 </a><tal:comma condition="not:repeat/distribution/end">, </tal:comma>25 </a><tal:comma condition="not:repeat/distribution/end">, </tal:comma>
23 </tal:distribution>26 </tal:distribution>
24 </div>27 </div>
2528
26 <div tal:condition="context/projects" style="margin-top:1em;">29 <div tal:condition="projectgroups" style="margin-top:1em;">
27 <h3 style="display: inline;">Project groups:</h3>30 <h3 style="display: inline;">Project groups:</h3>
28 <tal:project31 <tal:projectgroup repeat="projectgroup projectgroups">
29 repeat="projectgroup context/projects">
30 <a href="#" tal:replace="structure projectgroup/fmt:link">GNOME32 <a href="#" tal:replace="structure projectgroup/fmt:link">GNOME
31 </a><tal:comma condition="not:repeat/projectgroup/end">, </tal:comma>33 </a><tal:comma condition="not:repeat/projectgroup/end">, </tal:comma>
34 </tal:projectgroup>
35 </div>
36
37 <div tal:condition="projects" style="margin-top:1em;">
38 <h3 style="display: inline;">Projects:</h3>
39 <tal:project repeat="project projects">
40 <a href="#" tal:replace="structure project/fmt:link">Firefox
41 </a><tal:comma condition="not:repeat/project/end">, </tal:comma>
32 </tal:project>42 </tal:project>
33 </div>43 </div>
3444
35 <div tal:condition="context/products" style="margin-top:1em;">
36 <h3 style="display: inline;">Projects:</h3>
37 <tal:product
38 repeat="product context/products">
39 <a href="#" tal:replace="structure product/fmt:link">Firefox
40 </a><tal:comma condition="not:repeat/product/end">, </tal:comma>
41 </tal:product>
42 </div>
43
44 </div>45 </div>
45</tal:root>46</tal:root>