launchpad-reviewers team mailing list archive

[Steve Kowalik <stevenk@xxxxxxxxxx>]

Steve Kowalik has proposed merging lp:~stevenk/launchpad/bugtrackers-for-private-projects into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1174542 in Launchpad itself: "Bug tracker linked to private project causes BugTrackerSet:+index to 403"
  https://bugs.launchpad.net/launchpad/+bug/1174542

For more details, see:
https://code.launchpad.net/~stevenk/launchpad/bugtrackers-for-private-projects/+merge/161715

Do not show bug trackers that are linked to private projects that the user can not see.
-- 
https://code.launchpad.net/~stevenk/launchpad/bugtrackers-for-private-projects/+merge/161715
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stevenk/launchpad/bugtrackers-for-private-projects into lp:launchpad.

=== modified file 'lib/lp/bugs/browser/bugtracker.py'
--- lib/lp/bugs/browser/bugtracker.py	2013-04-10 08:09:05 +0000
+++ lib/lp/bugs/browser/bugtracker.py	2013-04-30 22:11:31 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2011 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2013 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Bug tracker views."""
@@ -167,7 +167,7 @@
         # bug watch counts per tracker. However the batching makes
         # the inefficiency tolerable for now. Robert Collins 20100919.
         self._pillar_cache = self.context.getPillarsForBugtrackers(
-            list(self.context.trackers()))
+            list(self.context.trackers(user=self.user)))
 
     @property
     def inactive_tracker_count(self):
@@ -175,14 +175,14 @@
 
     @cachedproperty
     def active_trackers(self):
-        results = self.context.trackers(active=True)
+        results = self.context.trackers(user=self.user, active=True)
         navigator = ActiveBatchNavigator(results, self.request)
         navigator.setHeadings('tracker', 'trackers')
         return navigator
 
     @cachedproperty
     def inactive_trackers(self):
-        results = self.context.trackers(active=False)
+        results = self.context.trackers(user=self.user, active=False)
         navigator = InactiveBatchNavigator(results, self.request)
         navigator.setHeadings('tracker', 'trackers')
         return navigator

=== modified file 'lib/lp/bugs/browser/tests/test_bugtracker_views.py'
--- lib/lp/bugs/browser/tests/test_bugtracker_views.py	2013-01-03 00:27:37 +0000
+++ lib/lp/bugs/browser/tests/test_bugtracker_views.py	2013-04-30 22:11:31 +0000
@@ -1,4 +1,4 @@
-# Copyright 2010-2011 Canonical Ltd.  This software is licensed under the
+# Copyright 2010-2013 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Tests for BugTracker views."""
@@ -7,16 +7,15 @@
 
 from zope.component import getUtility
 
-from lp.app.interfaces.launchpad import ILaunchpadCelebrities
+from lp.app.enums import InformationType
 from lp.bugs.interfaces.bugtracker import IBugTrackerSet
 from lp.services.webapp import canonical_url
 from lp.testing import (
-    person_logged_in,
+    celebrity_logged_in,
     TestCaseWithFactory,
     )
 from lp.testing.layers import DatabaseFunctionalLayer
 from lp.testing.matchers import IsConfiguredBatchNavigator
-from lp.testing.sampledata import ADMIN_EMAIL
 from lp.testing.views import create_initialized_view
 
 
@@ -29,8 +28,7 @@
         tracker = self.factory.makeBugTracker()
         project_group = self.factory.makeProject() 
         product = self.factory.makeProduct()
-        admin = getUtility(ILaunchpadCelebrities).admin.teamowner
-        with person_logged_in(admin):
+        with celebrity_logged_in('admin'):
             project_group.bugtracker = tracker
             product.bugtracker = tracker
         view = create_initialized_view(tracker, name='+index')
@@ -41,8 +39,7 @@
         tracker = self.factory.makeBugTracker()
         active_product = self.factory.makeProduct()
         inactive_product = self.factory.makeProduct()
-        admin = getUtility(ILaunchpadCelebrities).admin.teamowner
-        with person_logged_in(admin):
+        with celebrity_logged_in('admin'):
             active_product.bugtracker = tracker
             inactive_product.bugtracker = tracker
             inactive_product.active = False
@@ -62,12 +59,11 @@
         self.assertThat(view.inactive_trackers, matcher)
 
     def test_page_is_batched(self):
-        active_tracker1 = self.factory.makeBugTracker()
-        active_tracker2 = self.factory.makeBugTracker()
+        self.factory.makeBugTracker()
+        self.factory.makeBugTracker()
         inactive_tracker1 = self.factory.makeBugTracker()
         inactive_tracker2 = self.factory.makeBugTracker()
-        admin = getUtility(ILaunchpadCelebrities).admin.teamowner
-        with person_logged_in(admin):
+        with celebrity_logged_in('admin'):
             inactive_tracker1.active = False
             inactive_tracker2.active = False
         trackers = getUtility(IBugTrackerSet)
@@ -83,3 +79,14 @@
         #    find_tag_by_id(content, 'lower-batch-nav-batchnav-next')['class'])
         # Instead we check the string appears.
         self.assertTrue('upper-batch-nav-batchnav-next' in content)
+
+    def test_tracker_with_private_project(self):
+        tracker = self.factory.makeBugTracker()
+        project = self.factory.makeProduct(
+            information_type=InformationType.PROPRIETARY)
+        with celebrity_logged_in('admin'):
+            project.bugtracker = tracker
+        url = canonical_url(getUtility(IBugTrackerSet))
+        browser = self.getUserBrowser(url)
+        self.assertIn('GnomeGBug GTracker', browser.contents)
+        self.assertNotIn(tracker.name, browser.contents)

=== modified file 'lib/lp/bugs/model/bugtracker.py'
--- lib/lp/bugs/model/bugtracker.py	2013-01-07 02:40:55 +0000
+++ lib/lp/bugs/model/bugtracker.py	2013-04-30 22:11:31 +0000
@@ -1,10 +1,9 @@
-# Copyright 2009-2011 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2013 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __metaclass__ = type
 __all__ = [
     'BugTracker',
-    'BugTrackerSet',
     'BugTrackerAlias',
     'BugTrackerAliasSet',
     'BugTrackerComponent',
@@ -74,11 +73,6 @@
     validate_public_person,
     )
 from lp.services.database.enumcol import EnumCol
-from lp.services.database.interfaces import (
-    DEFAULT_FLAVOR,
-    IStoreSelector,
-    MAIN_STORE,
-    )
 from lp.services.database.lpstorm import IStore
 from lp.services.database.sqlbase import (
     flush_database_updates,
@@ -779,19 +773,19 @@
         """See `IBugTrackerSet`."""
         return BugTracker.select()
 
-    def trackers(self, active=None):
-        # Without context, cannot tell what store flavour is desirable.
-        store = getUtility(IStoreSelector).get(MAIN_STORE, DEFAULT_FLAVOR)
-        if active is not None:
-            clauses = [BugTracker.active == active]
-        else:
-            clauses = []
-        results = store.find(BugTracker, *clauses)
-        results.order_by(BugTracker.name)
-        return results
+    def trackers(self, user=None, active=None):
+        # Circular.
+        from lp.registry.model.product import Product, ProductSet
+        clauses = [
+            Product.bugtracker == BugTracker.id,
+            ProductSet.getProductPrivacyFilter(user)]
+        if active:
+            clauses.append(BugTracker.active == active)
+        return IStore(BugTracker).find(BugTracker, *clauses).order_by(
+            BugTracker.name)
 
-    def ensureBugTracker(self, baseurl, owner, bugtrackertype,
-        title=None, summary=None, contactdetails=None, name=None):
+    def ensureBugTracker(self, baseurl, owner, bugtrackertype, title=None,
+                         summary=None, contactdetails=None, name=None):
         """See `IBugTrackerSet`."""
         # Try to find an existing bug tracker that matches.
         bugtracker = self.queryByBaseURL(baseurl)
@@ -826,16 +820,10 @@
 
     def getMostActiveBugTrackers(self, limit=None):
         """See `IBugTrackerSet`."""
-        store = IStore(BugTracker)
-        result = store.find(
+        return IStore(BugTracker).find(
             BugTracker,
-            BugTracker.id == BugWatch.bugtrackerID)
-        result = result.group_by(BugTracker)
-        result = result.order_by(Desc(Count(BugWatch)))
-        if limit is not None:
-            return result[:limit]
-        else:
-            return result
+            BugTracker.id == BugWatch.bugtrackerID).group_by(
+                BugTracker).order_by(Desc(Count(BugWatch))).config(limit=limit)
 
     def getPillarsForBugtrackers(self, bugtrackers):
         """See `IBugTrackerSet`."""