launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #26448
[Merge] ~pappacena/launchpad:snap-pillar-list-filters into launchpad:master
Thiago F. Pappacena has proposed merging ~pappacena/launchpad:snap-pillar-list-filters into launchpad:master with ~pappacena/launchpad:snap-pillar-subscribe-ui as a prerequisite.
Commit message:
Filtering for visible snaps on +snaps pages
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/398751
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~pappacena/launchpad:snap-pillar-list-filters into launchpad:master.
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 21a4eb6..13ce894 100644
--- a/lib/lp/snappy/model/snap.py
+++ b/lib/lp/snappy/model/snap.py
@@ -1436,7 +1436,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()
@@ -1446,6 +1447,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):
@@ -1463,8 +1465,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))
@@ -1479,28 +1483,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):
@@ -1508,16 +1519,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:
Follow ups
