Merge lp:~jcsackett/launchpad/filter-in-getRequestTargets into lp:launchpad

[j.c.sackett]

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

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

[Aaron Bentley (abentley)]

In getRequestTargets and list_product_request_targets, having the user default to None means that calling code can accidentally omit it. I think it should be a non-optional parameter, even though this means changing parameter order.

Other than that, this looks good to land.

review: Approve

=== 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 23:48:21 +0000
@@ -617,7 +617,11 @@
 
     def __call__(self, context):
         import_queue = getUtility(ITranslationImportQueue)
-        targets = import_queue.getRequestTargets()
+        if hasattr(self, 'view'):
+            user = self.view.user
+        else:
+            user = None
+        targets = import_queue.getRequestTargets(user)
         filtered_targets = set()
 
         # Read filter_status, in order to mark targets that have requests with
@@ -634,7 +638,8 @@
                 try:
                     status = RosettaImportStatus.items[status_filter]
                     filtered_targets = set(
-                        import_queue.getRequestTargets(status))
+                        import_queue.getRequestTargets(
+                            user=None, status=status))
                 except LookupError:
                     # Unknown status.  Ignore.
                     pass

=== modified file 'lib/lp/translations/doc/translationimportqueue.txt'
--- lib/lp/translations/doc/translationimportqueue.txt	2011-12-30 06:14:56 +0000
+++ lib/lp/translations/doc/translationimportqueue.txt	2012-11-21 23:48:21 +0000
@@ -22,7 +22,8 @@
     >>> def get_target_names(status=None):
     ...     """Call getRequestTargets, return list of names/titles."""
     ...     result = []
-    ...     queue = translationimportqueue.getRequestTargets(status)
+    ...     queue = translationimportqueue.getRequestTargets(
+    ...         user=None,status=status)
     ...     for object in queue:
     ...         if IDistroSeries.providedBy(object):
     ...             name = "%s/%s" % (object.distribution.name, object.name)

=== 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 23:48:21 +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(user,  status=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 23:48:21 +0000
@@ -848,7 +848,7 @@
         return elapsedtime_text
 
 
-def list_product_request_targets(status_condition):
+def list_product_request_targets(user, status_condition):
     """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, user, status=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(user, status_clause)
 
         return distroseries + products
 

=== modified file 'lib/lp/translations/scripts/po_import.py'
--- lib/lp/translations/scripts/po_import.py	2012-06-29 08:40:05 +0000
+++ lib/lp/translations/scripts/po_import.py	2012-11-21 23:48:21 +0000
@@ -163,7 +163,7 @@
         # We'll serve these queues in turn, one request each, until either the
         # queue is drained or our time is up.
         importqueues = translation_import_queue.getRequestTargets(
-            RosettaImportStatus.APPROVED)
+            user=None, status=RosettaImportStatus.APPROVED)
 
         if not importqueues:
             self.logger.info("No requests pending.")

=== 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 23:48:21 +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,
@@ -571,14 +572,25 @@
             sorted(names),
             [
                 product.name
-                for product in list_product_request_targets(True)])
+                for product in list_product_request_targets(None, 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(None, True))
+        self.assertEqual([product], list_product_request_targets(owner, True))
 
     def test_list_product_request_targets_ignores_distro_uploads(self):
         self.clearQueue()
         self.useFixture(FakeLibrarian())
         self.factory.makeTranslationImportQueueEntry(
             distroseries=self.factory.makeDistroSeries())
-        self.assertEqual([], list_product_request_targets(True))
+        self.assertEqual([], list_product_request_targets(None, True))
 
     def test_list_product_request_targets_ignores_inactive_products(self):
         self.clearQueue()
@@ -587,7 +599,7 @@
         product.active = False
         self.factory.makeTranslationImportQueueEntry(
             productseries=self.factory.makeProductSeries(product=product))
-        self.assertEqual([], list_product_request_targets(False))
+        self.assertEqual([], list_product_request_targets(None, False))
 
     def test_list_product_request_targets_does_not_duplicate(self):
         # list_product_request_targets will list a product only once.
@@ -601,7 +613,7 @@
             for counter in range(2):
                 self.factory.makeTranslationImportQueueEntry(
                     productseries=series)
-        self.assertEqual([product], list_product_request_targets(True))
+        self.assertEqual([product], list_product_request_targets(None, True))
 
     def test_list_product_request_targets_filters_status(self):
         self.clearQueue()
@@ -614,10 +626,12 @@
         self.assertEqual(
             [],
             list_product_request_targets(
+                None,
                 TranslationImportQueueEntry.status == other_status))
         self.assertEqual(
             [entry.productseries.product],
             list_product_request_targets(
+                None,
                 TranslationImportQueueEntry.status == entry_status))
 
     def test_list_distroseries_request_targets_orders_by_names(self):