launchpad-reviewers team mailing list archive

[Aaron Bentley <aaron@xxxxxxxxxxxxx>]

Aaron Bentley has proposed merging lp:~abentley/launchpad/class-alias-mergable into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~abentley/launchpad/class-alias-mergable/+merge/50940

= Summary =
Express the findMergeablePackagings operation more clearly.

== Proposed fix ==
Use ClassAlias instead of a sub-select.  The Storm bug 718864 which prevented
using ClassAlias before turns out to be easy to work around.

== Pre-implementation notes ==
None

== Implementation details ==
Using Reference members of ClassAliases exhibits the bug, but using the
corresponding id works fine.

== Tests ==
bin/test -t FindMergable translations

== Demo and Q/A ==
None

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/translations/translationmerger.py
-- 
https://code.launchpad.net/~abentley/launchpad/class-alias-mergable/+merge/50940
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~abentley/launchpad/class-alias-mergable into lp:launchpad.

=== modified file 'lib/lp/translations/translationmerger.py'
--- lib/lp/translations/translationmerger.py	2011-02-14 18:53:27 +0000
+++ lib/lp/translations/translationmerger.py	2011-02-23 15:45:27 +0000
@@ -12,7 +12,7 @@
 from operator import methodcaller
 
 from storm.locals import (
-    Select,
+    ClassAlias,
     Store,
     )
 from zope.component import getUtility
@@ -357,29 +357,17 @@
     def findMergeablePackagings():
         """Find packagings where both product and package have templates."""
         store = IStore(Packaging)
-        upstream_translated = Select(
-            Packaging.id,
-            Packaging.productseries == POTemplate.productseriesID,
-            )
+        PackageTemplate = ClassAlias(POTemplate, 'PackageTemplate')
         ubuntu = getUtility(ILaunchpadCelebrities).ubuntu
         result = store.find(
-            Packaging, Packaging.id.is_in(upstream_translated),
-            Packaging.distroseries == POTemplate.distroseriesID,
+            Packaging,
+            Packaging.productseries == POTemplate.productseriesID,
+            Packaging.distroseries == PackageTemplate.distroseriesID,
             Packaging.distroseries == DistroSeries.id,
             DistroSeries.distribution == ubuntu.id,
-            Packaging.sourcepackagename == POTemplate.sourcepackagenameID,
+            Packaging.sourcepackagename ==
+                PackageTemplate.sourcepackagenameID,
             )
-        # XXX: AaronBentley 2011-02-14 bug=718864: this should be possible
-        # without using a subselect, but a Storm ClassAlias bug prevents us
-        # from doing it this way:
-        # PackageTemplate = ClassAlias(POTemplate)
-        # result = store.find(
-        #     Packaging,
-        #     Packaging.productseries == POTemplate.productseries,
-        #     Packaging.distroseries == PackageTemplate.distroseries,
-        #     Packaging.sourcepackagename ==
-        #         PackageTemplate.sourcepackagename,
-        #     )
         result.config(distinct=True)
         return result