launchpad-reviewers team mailing list archive

["j.c.sackett" <jcsackett@xxxxxxxxxxxxx>]

j.c.sackett has proposed merging lp:~jcsackett/launchpad/filter-in-getRequestTargets into lp:launchpad.

Commit message:
Adds private product filtering to getRequestTargets in the translations import queue.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1081666 in Launchpad itself: "list_product_request does not filter private products"
  https://bugs.launchpad.net/launchpad/+bug/1081666

For more details, see:
https://code.launchpad.net/~jcsackett/launchpad/filter-in-getRequestTargets/+merge/135457

Summary
=======
The import queue isn't aware of private products, because the function driving
it, get_product_request_targets, is unaware. That function should be updated
to use the privacy filter provided by ProductSet.

Preimp
======
Spoke with Rick Harding about alternatives to ILaunchBag use.

Implementation
==============
getRequestTargets is updated to take a user argument, which it passes to
get_product_request_targets. The view provides this to the call.

get_product_request_targets is updated to get the privacy filter clause from
ProductSet.getProductPrivacyFilter, using the supplied user.

The interface is updated to include the new user argument, and supply the
REQUEST_USER as the argument for the API export.

Tests
=====
bin/test -vvct test_list_product_request_filters_private_products

QA
==
Ensure private products do not appear in the import queue.

LoC
===
Part of private projects.

Lint
====

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/translations/model/translationimportqueue.py
  lib/lp/translations/browser/translationimportqueue.py
  lib/lp/translations/tests/test_translationimportqueue.py
  lib/lp/translations/interfaces/translationimportqueue.py
-- 
https://code.launchpad.net/~jcsackett/launchpad/filter-in-getRequestTargets/+merge/135457
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jcsackett/launchpad/filter-in-getRequestTargets into lp:launchpad.

=== modified file 'lib/lp/translations/browser/translationimportqueue.py'
--- lib/lp/translations/browser/translationimportqueue.py	2011-12-30 06:14:56 +0000
+++ lib/lp/translations/browser/translationimportqueue.py	2012-11-21 16:01:22 +0000
@@ -617,7 +617,7 @@
 
     def __call__(self, context):
         import_queue = getUtility(ITranslationImportQueue)
-        targets = import_queue.getRequestTargets()
+        targets = import_queue.getRequestTargets(user=self.user)
         filtered_targets = set()
 
         # Read filter_status, in order to mark targets that have requests with

=== modified file 'lib/lp/translations/interfaces/translationimportqueue.py'
--- lib/lp/translations/interfaces/translationimportqueue.py	2011-12-24 16:54:44 +0000
+++ lib/lp/translations/interfaces/translationimportqueue.py	2012-11-21 16:01:22 +0000
@@ -400,7 +400,8 @@
     @operation_parameters(
         status=copy_field(ITranslationImportQueueEntry['status']))
     @operation_returns_collection_of(IHasTranslationImports)
-    def getRequestTargets(status=None):
+    @call_with(user=REQUEST_USER)
+    def getRequestTargets(status=None, user=None):
         """List `Product`s and `DistroSeries` with pending imports.
 
         :arg status: Filter by `RosettaImportStatus`.

=== modified file 'lib/lp/translations/model/translationimportqueue.py'
--- lib/lp/translations/model/translationimportqueue.py	2011-12-30 06:14:56 +0000
+++ lib/lp/translations/model/translationimportqueue.py	2012-11-21 16:01:22 +0000
@@ -848,7 +848,7 @@
         return elapsedtime_text
 
 
-def list_product_request_targets(status_condition):
+def list_product_request_targets(status_condition, user=None):
     """Return list of Products with import queue entries.
 
     :param status_condition: Storm conditional restricting the
@@ -856,9 +856,11 @@
     :return: A list of `Product`, distinct and ordered by name.
     """
     # Avoid circular imports.
-    from lp.registry.model.product import Product
+    from lp.registry.model.product import Product, ProductSet
     from lp.registry.model.productseries import ProductSeries
 
+    privacy_filter = ProductSet.getProductPrivacyFilter(user)
+
     products = IStore(Product).find(
         Product,
         Product.id == ProductSeries.productID,
@@ -868,7 +870,8 @@
             And(
                 TranslationImportQueueEntry.productseries_id != None,
                 status_condition),
-            distinct=True)))
+            distinct=True)),
+        privacy_filter)
 
     # Products may occur multiple times due to the join with
     # ProductSeries.
@@ -1281,7 +1284,7 @@
             " AND ".join(queries), clauseTables=clause_tables,
             orderBy=['dateimported'])
 
-    def getRequestTargets(self, status=None):
+    def getRequestTargets(self, status=None, user=None):
         """See `ITranslationImportQueue`."""
 
         if status is None:
@@ -1290,7 +1293,7 @@
             status_clause = (TranslationImportQueueEntry.status == status)
 
         distroseries = list_distroseries_request_targets(status_clause)
-        products = list_product_request_targets(status_clause)
+        products = list_product_request_targets(status_clause, user)
 
         return distroseries + products
 

=== modified file 'lib/lp/translations/tests/test_translationimportqueue.py'
--- lib/lp/translations/tests/test_translationimportqueue.py	2012-01-20 15:42:44 +0000
+++ lib/lp/translations/tests/test_translationimportqueue.py	2012-11-21 16:01:22 +0000
@@ -10,6 +10,7 @@
 from zope.component import getUtility
 from zope.security.proxy import removeSecurityProxy
 
+from lp.app.enums import InformationType
 from lp.app.interfaces.launchpad import ILaunchpadCelebrities
 from lp.services.database.lpstorm import (
     ISlaveStore,
@@ -573,6 +574,17 @@
                 product.name
                 for product in list_product_request_targets(True)])
 
+    def test_list_product_request_filters_private_products(self):
+        self.clearQueue()
+        self.useFixture(FakeLibrarian())
+        owner = self.factory.makePerson()
+        product = self.factory.makeProduct(owner=owner,
+            information_type=InformationType.PROPRIETARY)
+        self.factory.makeTranslationImportQueueEntry(
+            productseries=self.factory.makeProductSeries(product=product))
+        self.assertEqual([], list_product_request_targets(True))
+        self.assertEqual([product], list_product_request_targets(True, owner))
+
     def test_list_product_request_targets_ignores_distro_uploads(self):
         self.clearQueue()
         self.useFixture(FakeLibrarian())