Merge ~pappacena/launchpad:snap-pillar-list-filters into launchpad:master

[Thiago F. Pappacena]

Filtering for visible snaps on +snaps pages

94902ed... by Thiago F. Pappacena on 2021-03-02

Merge branch 'snap-pillar-subscribe-ui' into snap-pillar-list-filters

3742242... by Thiago F. Pappacena on 2021-03-03

Merge branch 'snap-pillar-subscribe-ui' into snap-pillar-list-filters

[Colin Watson (cjwatson)]

review: Approve

diff --git a/lib/lp/snappy/browser/tests/test_snaplisting.py b/lib/lp/snappy/browser/tests/test_snaplisting.py
index ce229c1..9216c91 100644
--- a/lib/lp/snappy/browser/tests/test_snaplisting.py
+++ b/lib/lp/snappy/browser/tests/test_snaplisting.py
@@ -1,4 +1,4 @@
-# Copyright 2015-2017 Canonical Ltd.  This software is licensed under the
+# Copyright 2015-2021 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Test snap package listings."""
@@ -27,7 +27,9 @@ from lp.services.database.constants import (
     SEVEN_DAYS_AGO,
     UTC_NOW,
     )
+from lp.services.features.testing import FeatureFixture
 from lp.services.webapp import canonical_url
+from lp.snappy.interfaces.snap import SNAP_TESTING_FLAGS
 from lp.testing import (
     ANONYMOUS,
     BrowserTestCase,
@@ -154,6 +156,204 @@ class TestSnapListing(BrowserTestCase):
             snap-name.*     Team Name.*     ~.*:.*          .*
             snap-name.*     Team Name.*     lp:.*           .*""", text)
 
+    def test_project_private_snap_listing(self):
+        # Only users with permission can see private snap packages in the list
+        # for a project.
+        self.useFixture(FeatureFixture(SNAP_TESTING_FLAGS))
+        project = self.factory.makeProduct(displayname="Snappable")
+        private_owner = self.factory.makePerson()
+        user_with_permission = self.factory.makePerson()
+        someone_else = self.factory.makePerson()
+        private_snap = self.factory.makeSnap(
+            name="private-snap",
+            private=True, registrant=private_owner, owner=private_owner,
+            branch=self.factory.makeProductBranch(product=project),
+            date_created=ONE_DAY_AGO)
+        with person_logged_in(private_owner):
+            private_snap.subscribe(user_with_permission, private_owner)
+        [ref] = self.factory.makeGitRefs(target=project)
+        self.factory.makeSnap(git_ref=ref, date_created=UTC_NOW)
+
+        full_list = """
+            Snap packages for Snappable
+            Name            Owner           Source          Registered
+            snap-name.*     Team Name.*     ~.*:.*           .*
+            private-snap.*  Person-name.*   lp:.*            .*"""
+
+        public_list = """
+            Snap packages for Snappable
+            Name            Owner           Source          Registered
+            snap-name.*     Team Name.*     ~.*:.*          .*"""
+
+        # private_owner: full_list.
+        self.assertTextMatchesExpressionIgnoreWhitespace(
+            full_list, self.getMainText(project, "+snaps", user=private_owner))
+        # user_with_permission: full_list.
+        self.assertTextMatchesExpressionIgnoreWhitespace(
+            full_list,
+            self.getMainText(project, "+snaps", user=user_with_permission))
+        # someone_else: public_list.
+        self.assertTextMatchesExpressionIgnoreWhitespace(
+            public_list,
+            self.getMainText(project, "+snaps", user=someone_else))
+        # Not logged in: public_list.
+        self.assertTextMatchesExpressionIgnoreWhitespace(
+            public_list, self.getMainText(project, "+snaps", user=None))
+
+    def test_person_private_snap_listing(self):
+        self.useFixture(FeatureFixture(SNAP_TESTING_FLAGS))
+        private_owner = self.factory.makePerson(name="random-user")
+        user_with_permission = self.factory.makePerson()
+        someone_else = self.factory.makePerson()
+        private_snap = self.factory.makeSnap(
+            name="private-snap",
+            private=True, registrant=private_owner, owner=private_owner,
+            date_created=ONE_DAY_AGO)
+        with person_logged_in(private_owner):
+            private_snap.subscribe(user_with_permission, private_owner)
+        [ref] = self.factory.makeGitRefs()
+        self.factory.makeSnap(
+            private=False, registrant=private_owner, owner=private_owner,
+            git_ref=ref, date_created=UTC_NOW)
+
+        full_list = """
+            Snap packages for Random-user
+            Name               Source          Registered
+            snap-name.*        ~.*:.*          .*
+            private-snap.*     lp:.*           .*"""
+
+        public_list = """
+            Snap packages for Random-user
+            Name               Source          Registered
+            snap-name.*        ~.*:.*          .*"""
+
+        # private_owner: full_list.
+        self.assertTextMatchesExpressionIgnoreWhitespace(
+            full_list, self.getMainText(private_owner, "+snaps",
+                                        user=private_owner))
+        # user_with_permission: full_list.
+        self.assertTextMatchesExpressionIgnoreWhitespace(
+            full_list, self.getMainText(private_owner, "+snaps",
+                                        user=user_with_permission))
+        # someone_else: public_list.
+        self.assertTextMatchesExpressionIgnoreWhitespace(
+            public_list, self.getMainText(private_owner, "+snaps",
+                                          user=someone_else))
+        # Not logged in: public_list.
+        self.assertTextMatchesExpressionIgnoreWhitespace(
+            public_list, self.getMainText(private_owner, "+snaps", user=None))
+
+    def test_branch_private_snap_listing(self):
+        # Only certain users can see private snaps on branch listing.
+        self.useFixture(FeatureFixture(SNAP_TESTING_FLAGS))
+        private_owner = self.factory.makePerson(name="random-user")
+        user_with_permission = self.factory.makePerson()
+        someone_else = self.factory.makePerson()
+        branch = self.factory.makeAnyBranch()
+        private_snap = self.factory.makeSnap(
+            private=True, name="private-snap",
+            owner=private_owner, registrant=private_owner, branch=branch)
+        with person_logged_in(private_owner):
+            private_snap.subscribe(user_with_permission, private_owner)
+        self.factory.makeSnap(
+            private=False, owner=private_owner, registrant=private_owner,
+            branch=branch)
+        full_list = """
+            Snap packages for lp:.*
+            Name            Owner           Registered
+            snap-name.*     Random-user.*   .*
+            private-snap.*  Random-user.*   .*"""
+        public_list = """
+            Snap packages for lp:.*
+            Name            Owner           Registered
+            snap-name.*     Random-user.*     .*"""
+
+        self.assertTextMatchesExpressionIgnoreWhitespace(
+            full_list, self.getMainText(branch, "+snaps", user=private_owner))
+        self.assertTextMatchesExpressionIgnoreWhitespace(
+            full_list, self.getMainText(branch, "+snaps",
+                                        user=user_with_permission))
+        self.assertTextMatchesExpressionIgnoreWhitespace(
+            public_list, self.getMainText(branch, "+snaps", user=someone_else))
+        self.assertTextMatchesExpressionIgnoreWhitespace(
+            public_list, self.getMainText(branch, "+snaps", user=None))
+
+    def test_git_repository_private_snap_listing(self):
+        # Only certain users can see private snaps on git repo listing.
+        self.useFixture(FeatureFixture(SNAP_TESTING_FLAGS))
+        private_owner = self.factory.makePerson(name="random-user")
+        user_with_permission = self.factory.makePerson()
+        someone_else = self.factory.makePerson()
+        repository = self.factory.makeGitRepository()
+        [ref] = self.factory.makeGitRefs(repository=repository)
+        private_snap = self.factory.makeSnap(
+            private=True, name="private-snap",
+            owner=private_owner, registrant=private_owner, git_ref=ref)
+        with person_logged_in(private_owner):
+            private_snap.subscribe(user_with_permission, private_owner)
+        self.factory.makeSnap(
+            private=False, owner=private_owner, registrant=private_owner,
+            git_ref=ref)
+
+        full_list = """
+            Snap packages for lp:~.*
+            Name            Owner           Registered
+            snap-name.*     Random-user.*   .*
+            private-snap.*  Random-user.*   .*"""
+        public_list = """
+            Snap packages for lp:.*
+            Name            Owner           Registered
+            snap-name.*     Random-user.*     .*"""
+
+        self.assertTextMatchesExpressionIgnoreWhitespace(
+            full_list,
+            self.getMainText(repository, "+snaps", user=private_owner))
+        self.assertTextMatchesExpressionIgnoreWhitespace(
+            full_list,
+            self.getMainText(repository, "+snaps", user=user_with_permission))
+        self.assertTextMatchesExpressionIgnoreWhitespace(
+            public_list,
+            self.getMainText(repository, "+snaps", user=someone_else))
+        self.assertTextMatchesExpressionIgnoreWhitespace(
+            public_list, self.getMainText(repository, "+snaps", user=None))
+
+    def test_git_ref_private_snap_listing(self):
+        # Only certain users can see private snaps on git ref listing.
+        self.useFixture(FeatureFixture(SNAP_TESTING_FLAGS))
+        private_owner = self.factory.makePerson(name="random-user")
+        user_with_permission = self.factory.makePerson()
+        someone_else = self.factory.makePerson()
+        repository = self.factory.makeGitRepository()
+        [ref] = self.factory.makeGitRefs(repository=repository)
+        private_snap = self.factory.makeSnap(
+            private=True, name="private-snap",
+            owner=private_owner, registrant=private_owner, git_ref=ref)
+        with person_logged_in(private_owner):
+            private_snap.subscribe(user_with_permission, private_owner)
+        self.factory.makeSnap(
+            private=False, owner=private_owner, registrant=private_owner,
+            git_ref=ref)
+
+        full_list = """
+                    Snap packages for ~.*:.*
+                    Name            Owner           Registered
+                    snap-name.*     Random-user.*   .*
+                    private-snap.*  Random-user.*   .*"""
+        public_list = """
+                    Snap packages for ~.*:.*
+                    Name            Owner           Registered
+                    snap-name.*     Random-user.*     .*"""
+
+        self.assertTextMatchesExpressionIgnoreWhitespace(
+            full_list, self.getMainText(ref, "+snaps", user=private_owner))
+        self.assertTextMatchesExpressionIgnoreWhitespace(
+            full_list,
+            self.getMainText(ref, "+snaps", user=user_with_permission))
+        self.assertTextMatchesExpressionIgnoreWhitespace(
+            public_list, self.getMainText(ref, "+snaps", user=someone_else))
+        self.assertTextMatchesExpressionIgnoreWhitespace(
+            public_list, self.getMainText(ref, "+snaps", user=None))
+
     def assertSnapsQueryCount(self, context, item_creator):
         self.pushConfig("launchpad", default_batch_size=10)
         recorder1, recorder2 = record_two_runs(
diff --git a/lib/lp/snappy/model/snap.py b/lib/lp/snappy/model/snap.py
index 287ccc3..b101e18 100644
--- a/lib/lp/snappy/model/snap.py
+++ b/lib/lp/snappy/model/snap.py
@@ -1438,7 +1438,8 @@ class SnapSet:
             raise NoSuchSnap(name)
         return snap
 
-    def _getSnapsFromCollection(self, collection, owner=None):
+    def _getSnapsFromCollection(self, collection, owner=None,
+                                visible_by_user=None):
         if IBranchCollection.providedBy(collection):
             id_column = Snap.branch_id
             ids = collection.getBranchIds()
@@ -1448,6 +1449,7 @@ class SnapSet:
         expressions = [id_column.is_in(ids._get_select())]
         if owner is not None:
             expressions.append(Snap.owner == owner)
+        expressions.append(get_snap_privacy_filter(visible_by_user))
         return IStore(Snap).find(Snap, *expressions)
 
     def findByIds(self, snap_ids, visible_by_user=None):
@@ -1465,8 +1467,10 @@ class SnapSet:
         """See `ISnapSet`."""
         def _getSnaps(collection):
             collection = collection.visibleByUser(visible_by_user)
-            owned = self._getSnapsFromCollection(collection.ownedBy(person))
-            packaged = self._getSnapsFromCollection(collection, owner=person)
+            owned = self._getSnapsFromCollection(
+                collection.ownedBy(person), visible_by_user=visible_by_user)
+            packaged = self._getSnapsFromCollection(
+                collection, owner=person, visible_by_user=visible_by_user)
             return owned.union(packaged)
 
         bzr_collection = removeSecurityProxy(getUtility(IAllBranches))
@@ -1481,28 +1485,35 @@ class SnapSet:
         """See `ISnapSet`."""
         def _getSnaps(collection):
             return self._getSnapsFromCollection(
-                collection.visibleByUser(visible_by_user))
+                collection.visibleByUser(visible_by_user),
+                visible_by_user=visible_by_user)
 
         bzr_collection = removeSecurityProxy(IBranchCollection(project))
         git_collection = removeSecurityProxy(IGitCollection(project))
         return _getSnaps(bzr_collection).union(_getSnaps(git_collection))
 
-    def findByBranch(self, branch):
+    def findByBranch(self, branch, visible_by_user=None):
         """See `ISnapSet`."""
-        return IStore(Snap).find(Snap, Snap.branch == branch)
+        return IStore(Snap).find(
+            Snap,
+            Snap.branch == branch,
+            get_snap_privacy_filter(visible_by_user))
 
-    def findByGitRepository(self, repository, paths=None):
+    def findByGitRepository(self, repository, paths=None,
+                            visible_by_user=None):
         """See `ISnapSet`."""
         clauses = [Snap.git_repository == repository]
         if paths is not None:
             clauses.append(Snap.git_path.is_in(paths))
+        clauses.append(get_snap_privacy_filter(visible_by_user))
         return IStore(Snap).find(Snap, *clauses)
 
-    def findByGitRef(self, ref):
+    def findByGitRef(self, ref, visible_by_user=None):
         """See `ISnapSet`."""
         return IStore(Snap).find(
             Snap,
-            Snap.git_repository == ref.repository, Snap.git_path == ref.path)
+            Snap.git_repository == ref.repository, Snap.git_path == ref.path,
+            get_snap_privacy_filter(visible_by_user))
 
     def findByContext(self, context, visible_by_user=None, order_by_date=True):
         if IPerson.providedBy(context):
@@ -1510,16 +1521,13 @@ class SnapSet:
         elif IProduct.providedBy(context):
             snaps = self.findByProject(
                 context, visible_by_user=visible_by_user)
-        # XXX cjwatson 2015-09-15: At the moment we can assume that if you
-        # can see the source context then you can see the snap packages
-        # based on it.  This will cease to be true if snap packages gain
-        # privacy of their own.
         elif IBranch.providedBy(context):
-            snaps = self.findByBranch(context)
+            snaps = self.findByBranch(context, visible_by_user=visible_by_user)
         elif IGitRepository.providedBy(context):
-            snaps = self.findByGitRepository(context)
+            snaps = self.findByGitRepository(
+                context, visible_by_user=visible_by_user)
         elif IGitRef.providedBy(context):
-            snaps = self.findByGitRef(context)
+            snaps = self.findByGitRef(context, visible_by_user=visible_by_user)
         else:
             raise BadSnapSearchContext(context)
         if order_by_date: