launchpad-reviewers team mailing list archive

[Richard Harding <rick.harding@xxxxxxxxxxxxx>]

Richard Harding has proposed merging lp:~rharding/launchpad/rollback_16295 into lp:launchpad.

Commit message:
Rollback r16295 that added private project filters to queries around translations since private projects cannot have translations.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~rharding/launchpad/rollback_16295/+merge/136216

Rollback r16295 since we're making sure that private projects can't have translations. This revision added filters to queries to make sure that translations specific queries didn't fetch projects that were non-public. This is just going to add overhead to queries since we're locking those projects out of having translations all together.
-- 
https://code.launchpad.net/~rharding/launchpad/rollback_16295/+merge/136216
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rharding/launchpad/rollback_16295 into lp:launchpad.

=== modified file 'lib/lp/registry/model/product.py'
--- lib/lp/registry/model/product.py	2012-11-26 08:33:03 +0000
+++ lib/lp/registry/model/product.py	2012-11-26 16:33:19 +0000
@@ -299,7 +299,7 @@
     def composeLicensesColumn(cls, for_class=None):
         """Compose a Storm column specification for licences.
 
-        Use this to render a list of `Product` links without querying
+        Use this to render a list of `Product` linkes without querying
         licences for each one individually.
 
         It lets you prefetch the licensing information in the same

=== modified file 'lib/lp/translations/browser/translationgroup.py'
--- lib/lp/translations/browser/translationgroup.py	2012-11-20 18:17:26 +0000
+++ lib/lp/translations/browser/translationgroup.py	2012-11-26 16:33:19 +0000
@@ -25,7 +25,6 @@
     )
 from lp.app.errors import NotFoundError
 from lp.registry.browser.objectreassignment import ObjectReassignmentView
-from lp.services.propertycache import cachedproperty
 from lp.services.webapp import (
     canonical_url,
     GetitemNavigation,
@@ -73,18 +72,6 @@
         self.translation_groups = getUtility(ITranslationGroupSet)
         self.user_can_edit = check_permission('launchpad.Edit', self.context)
 
-    @cachedproperty
-    def distributions(self):
-        return self.context.fetchDistrosForDisplay()
-
-    @cachedproperty
-    def projectgroups(self):
-        return self.context.fetchProjectGroupsForDisplay()
-
-    @cachedproperty
-    def projects(self):
-        return self.context.fetchProjectsForDisplay(self.user)
-
     @property
     def label(self):
         return "%s translation group" % self.context.title

=== modified file 'lib/lp/translations/interfaces/translationgroup.py'
--- lib/lp/translations/interfaces/translationgroup.py	2012-11-20 18:17:26 +0000
+++ lib/lp/translations/interfaces/translationgroup.py	2012-11-26 16:33:19 +0000
@@ -138,7 +138,7 @@
             ordered by language name in English.
         """
 
-    def fetchProjectsForDisplay(user):
+    def fetchProjectsForDisplay():
         """Fetch `Product`s using this group, for display purposes.
 
         Prefetches display-related properties.

=== modified file 'lib/lp/translations/model/translationgroup.py'
--- lib/lp/translations/model/translationgroup.py	2012-11-26 08:33:03 +0000
+++ lib/lp/translations/model/translationgroup.py	2012-11-26 16:33:19 +0000
@@ -23,7 +23,6 @@
     LeftJoin,
     )
 from storm.store import Store
-from zope.component import getUtility
 from zope.interface import implements
 
 from lp.app.errors import NotFoundError
@@ -42,7 +41,6 @@
     LibraryFileAlias,
     LibraryFileContent,
     )
-from lp.services.webapp.interfaces import ILaunchBag
 from lp.services.worlddata.model.language import Language
 from lp.translations.interfaces.translationgroup import (
     ITranslationGroup,
@@ -111,17 +109,9 @@
     def products(self):
         """See `ITranslationGroup`."""
         # Avoid circular imports.
-        from lp.registry.model.product import (
-            Product,
-            ProductSet,
-        )
-        user = getUtility(ILaunchBag).user
-        results = Store.of(self).find(
-            Product,
-            Product.active==True,
-            Product.translationgroup==self.id,
-            ProductSet.getProductPrivacyFilter(user))
-        return results
+        from lp.registry.model.product import Product
+
+        return Product.selectBy(translationgroup=self.id, active=True)
 
     @property
     def projects(self):
@@ -192,20 +182,17 @@
         mapper = lambda row: row[slice(0, 3)]
         return DecoratedResultSet(translator_data, mapper)
 
-    def fetchProjectsForDisplay(self, user):
+    def fetchProjectsForDisplay(self):
         """See `ITranslationGroup`."""
         # Avoid circular imports.
         from lp.registry.model.product import (
             Product,
-            ProductSet,
             ProductWithLicenses,
             )
 
         using = [
             Product,
-            LeftJoin(
-                LibraryFileAlias,
-                LibraryFileAlias.id == Product.iconID),
+            LeftJoin(LibraryFileAlias, LibraryFileAlias.id == Product.iconID),
             LeftJoin(
                 LibraryFileContent,
                 LibraryFileContent.id == LibraryFileAlias.contentID),
@@ -218,9 +205,7 @@
             )
         product_data = ISlaveStore(Product).using(*using).find(
             columns,
-            Product.translationgroup == self.id,
-            Product.active == True,
-            ProductSet.getProductPrivacyFilter(user))
+            Product.translationgroupID == self.id, Product.active == True)
         product_data = product_data.order_by(Product.displayname)
 
         return [

=== modified file 'lib/lp/translations/templates/translationgroup-portlet-projects.pt'
--- lib/lp/translations/templates/translationgroup-portlet-projects.pt	2012-11-20 18:17:26 +0000
+++ lib/lp/translations/templates/translationgroup-portlet-projects.pt	2012-11-26 16:33:19 +0000
@@ -13,26 +13,30 @@
     <a href="#teams">translation teams</a>.
   </p>
 
-  <div id="related-projects">
-    <div tal:condition="view/distributions" style="margin-top:1em;">
+  <div id="related-projects"
+       tal:define="
+        distributions context/fetchDistrosForDisplay;
+        projectgroups context/fetchProjectGroupsForDisplay;
+        projects context/fetchProjectsForDisplay">
+    <div tal:condition="distributions" style="margin-top:1em;">
       <h3 style="display: inline;">Distributions:</h3>
-      <tal:distribution repeat="distribution view/distributions">
+      <tal:distribution repeat="distribution distributions">
         <a href="#" tal:replace="structure distribution/fmt:link">Ubuntu
         </a><tal:comma condition="not:repeat/distribution/end">, </tal:comma>
       </tal:distribution>
     </div>
 
-    <div tal:condition="view/projectgroups" style="margin-top:1em;">
+    <div tal:condition="projectgroups" style="margin-top:1em;">
       <h3 style="display: inline;">Project groups:</h3>
-      <tal:projectgroup repeat="projectgroup view/projectgroups">
+      <tal:projectgroup repeat="projectgroup projectgroups">
         <a href="#" tal:replace="structure projectgroup/fmt:link">GNOME
         </a><tal:comma condition="not:repeat/projectgroup/end">, </tal:comma>
       </tal:projectgroup>
     </div>
 
-    <div tal:condition="view/projects" style="margin-top:1em;">
+    <div tal:condition="projects" style="margin-top:1em;">
       <h3 style="display: inline;">Projects:</h3>
-      <tal:project repeat="project view/projects">
+      <tal:project repeat="project projects">
         <a href="#" tal:replace="structure project/fmt:link">Firefox
         </a><tal:comma condition="not:repeat/project/end">, </tal:comma>
       </tal:project>

=== modified file 'lib/lp/translations/tests/test_translationgroup.py'
--- lib/lp/translations/tests/test_translationgroup.py	2012-11-20 18:17:26 +0000
+++ lib/lp/translations/tests/test_translationgroup.py	2012-11-26 16:33:19 +0000
@@ -9,13 +9,11 @@
 import transaction
 from zope.component import getUtility
 
-from lp.app.enums import InformationType
 from lp.registry.interfaces.teammembership import (
     ITeamMembershipSet,
     TeamMembershipStatus,
     )
 from lp.testing import (
-    person_logged_in,
     TestCaseWithFactory,
     WebServiceTestCase,
     )
@@ -23,65 +21,6 @@
 from lp.translations.interfaces.translationgroup import ITranslationGroupSet
 
 
-class TestTranslationGroup(TestCaseWithFactory):
-
-    layer = ZopelessDatabaseLayer
-
-    def _setup_products(self):
-        """Helper to setup one public one non-public product."""
-        user = self.factory.makePerson()
-        private_owner = self.factory.makePerson()
-        group = self.factory.makeTranslationGroup()
-
-        with person_logged_in(user):
-            public_product = self.factory.makeProduct()
-            public_product.translationgroup = group
-
-        with person_logged_in(private_owner):
-            private_product = self.factory.makeProduct(
-                information_type=InformationType.PROPRIETARY,
-                owner=private_owner)
-            private_product.translationgroup = group
-
-        return group, user, private_owner
-
-    def test_non_public_products_hidden(self):
-        """Non Public products are not returned via products attribute."""
-        group, public_user, private_user = self._setup_products()
-
-        with person_logged_in(public_user):
-            self.assertEqual(
-                1,
-                group.products.count(),
-                'There is only one public product for this user')
-
-        with person_logged_in(private_user):
-            self.assertEqual(
-                2,
-                group.products.count(),
-                'There are two for the private user.')
-
-    def test_non_public_products_hidden_for_display(self):
-        """Non Public products are not returned via fetchProjectsForDisplay."""
-        group, public_user, private_user = self._setup_products()
-
-        # Magical transaction so our data shows up via ISlaveStore
-        import transaction
-        transaction.commit()
-
-        with person_logged_in(public_user):
-            self.assertEqual(
-                1,
-                len(group.fetchProjectsForDisplay(public_user)),
-                'There is only one public product for the public user')
-
-        with person_logged_in(private_user):
-            self.assertEqual(
-                2,
-                len(group.fetchProjectsForDisplay(private_user)),
-                'Both products show for the private user.')
-
-
 class TestTranslationGroupSet(TestCaseWithFactory):
     layer = ZopelessDatabaseLayer