Merge ~pappacena/launchpad:snap-pillar-list-filters into launchpad:master
- Git
- lp:~pappacena/launchpad
- snap-pillar-list-filters
- Merge into master
Proposed by
Thiago F. Pappacena
Status: | Merged | ||||
---|---|---|---|---|---|
Approved by: | Thiago F. Pappacena | ||||
Approved revision: | 3742242858c44d9b9733e109be0c851afda3ba14 | ||||
Merge reported by: | Otto Co-Pilot | ||||
Merged at revision: | not available | ||||
Proposed branch: | ~pappacena/launchpad:snap-pillar-list-filters | ||||
Merge into: | launchpad:master | ||||
Prerequisite: | ~pappacena/launchpad:snap-pillar-subscribe-ui | ||||
Diff against target: |
323 lines (+225/-17) 2 files modified
lib/lp/snappy/browser/tests/test_snaplisting.py (+201/-1) lib/lp/snappy/model/snap.py (+24/-16) |
||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Colin Watson (community) | Approve | ||
Review via email: mp+398751@code.launchpad.net |
Commit message
Filtering for visible snaps on +snaps pages
Description of the change
To post a comment you must log in.
- 94902ed... by Thiago F. Pappacena
-
Merge branch 'snap-pillar-
subscribe- ui' into snap-pillar- list-filters - 3742242... by Thiago F. Pappacena
-
Merge branch 'snap-pillar-
subscribe- ui' into snap-pillar- list-filters
Revision history for this message
Colin Watson (cjwatson) : | # |
review:
Approve
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | diff --git a/lib/lp/snappy/browser/tests/test_snaplisting.py b/lib/lp/snappy/browser/tests/test_snaplisting.py |
2 | index ce229c1..9216c91 100644 |
3 | --- a/lib/lp/snappy/browser/tests/test_snaplisting.py |
4 | +++ b/lib/lp/snappy/browser/tests/test_snaplisting.py |
5 | @@ -1,4 +1,4 @@ |
6 | -# Copyright 2015-2017 Canonical Ltd. This software is licensed under the |
7 | +# Copyright 2015-2021 Canonical Ltd. This software is licensed under the |
8 | # GNU Affero General Public License version 3 (see the file LICENSE). |
9 | |
10 | """Test snap package listings.""" |
11 | @@ -27,7 +27,9 @@ from lp.services.database.constants import ( |
12 | SEVEN_DAYS_AGO, |
13 | UTC_NOW, |
14 | ) |
15 | +from lp.services.features.testing import FeatureFixture |
16 | from lp.services.webapp import canonical_url |
17 | +from lp.snappy.interfaces.snap import SNAP_TESTING_FLAGS |
18 | from lp.testing import ( |
19 | ANONYMOUS, |
20 | BrowserTestCase, |
21 | @@ -154,6 +156,204 @@ class TestSnapListing(BrowserTestCase): |
22 | snap-name.* Team Name.* ~.*:.* .* |
23 | snap-name.* Team Name.* lp:.* .*""", text) |
24 | |
25 | + def test_project_private_snap_listing(self): |
26 | + # Only users with permission can see private snap packages in the list |
27 | + # for a project. |
28 | + self.useFixture(FeatureFixture(SNAP_TESTING_FLAGS)) |
29 | + project = self.factory.makeProduct(displayname="Snappable") |
30 | + private_owner = self.factory.makePerson() |
31 | + user_with_permission = self.factory.makePerson() |
32 | + someone_else = self.factory.makePerson() |
33 | + private_snap = self.factory.makeSnap( |
34 | + name="private-snap", |
35 | + private=True, registrant=private_owner, owner=private_owner, |
36 | + branch=self.factory.makeProductBranch(product=project), |
37 | + date_created=ONE_DAY_AGO) |
38 | + with person_logged_in(private_owner): |
39 | + private_snap.subscribe(user_with_permission, private_owner) |
40 | + [ref] = self.factory.makeGitRefs(target=project) |
41 | + self.factory.makeSnap(git_ref=ref, date_created=UTC_NOW) |
42 | + |
43 | + full_list = """ |
44 | + Snap packages for Snappable |
45 | + Name Owner Source Registered |
46 | + snap-name.* Team Name.* ~.*:.* .* |
47 | + private-snap.* Person-name.* lp:.* .*""" |
48 | + |
49 | + public_list = """ |
50 | + Snap packages for Snappable |
51 | + Name Owner Source Registered |
52 | + snap-name.* Team Name.* ~.*:.* .*""" |
53 | + |
54 | + # private_owner: full_list. |
55 | + self.assertTextMatchesExpressionIgnoreWhitespace( |
56 | + full_list, self.getMainText(project, "+snaps", user=private_owner)) |
57 | + # user_with_permission: full_list. |
58 | + self.assertTextMatchesExpressionIgnoreWhitespace( |
59 | + full_list, |
60 | + self.getMainText(project, "+snaps", user=user_with_permission)) |
61 | + # someone_else: public_list. |
62 | + self.assertTextMatchesExpressionIgnoreWhitespace( |
63 | + public_list, |
64 | + self.getMainText(project, "+snaps", user=someone_else)) |
65 | + # Not logged in: public_list. |
66 | + self.assertTextMatchesExpressionIgnoreWhitespace( |
67 | + public_list, self.getMainText(project, "+snaps", user=None)) |
68 | + |
69 | + def test_person_private_snap_listing(self): |
70 | + self.useFixture(FeatureFixture(SNAP_TESTING_FLAGS)) |
71 | + private_owner = self.factory.makePerson(name="random-user") |
72 | + user_with_permission = self.factory.makePerson() |
73 | + someone_else = self.factory.makePerson() |
74 | + private_snap = self.factory.makeSnap( |
75 | + name="private-snap", |
76 | + private=True, registrant=private_owner, owner=private_owner, |
77 | + date_created=ONE_DAY_AGO) |
78 | + with person_logged_in(private_owner): |
79 | + private_snap.subscribe(user_with_permission, private_owner) |
80 | + [ref] = self.factory.makeGitRefs() |
81 | + self.factory.makeSnap( |
82 | + private=False, registrant=private_owner, owner=private_owner, |
83 | + git_ref=ref, date_created=UTC_NOW) |
84 | + |
85 | + full_list = """ |
86 | + Snap packages for Random-user |
87 | + Name Source Registered |
88 | + snap-name.* ~.*:.* .* |
89 | + private-snap.* lp:.* .*""" |
90 | + |
91 | + public_list = """ |
92 | + Snap packages for Random-user |
93 | + Name Source Registered |
94 | + snap-name.* ~.*:.* .*""" |
95 | + |
96 | + # private_owner: full_list. |
97 | + self.assertTextMatchesExpressionIgnoreWhitespace( |
98 | + full_list, self.getMainText(private_owner, "+snaps", |
99 | + user=private_owner)) |
100 | + # user_with_permission: full_list. |
101 | + self.assertTextMatchesExpressionIgnoreWhitespace( |
102 | + full_list, self.getMainText(private_owner, "+snaps", |
103 | + user=user_with_permission)) |
104 | + # someone_else: public_list. |
105 | + self.assertTextMatchesExpressionIgnoreWhitespace( |
106 | + public_list, self.getMainText(private_owner, "+snaps", |
107 | + user=someone_else)) |
108 | + # Not logged in: public_list. |
109 | + self.assertTextMatchesExpressionIgnoreWhitespace( |
110 | + public_list, self.getMainText(private_owner, "+snaps", user=None)) |
111 | + |
112 | + def test_branch_private_snap_listing(self): |
113 | + # Only certain users can see private snaps on branch listing. |
114 | + self.useFixture(FeatureFixture(SNAP_TESTING_FLAGS)) |
115 | + private_owner = self.factory.makePerson(name="random-user") |
116 | + user_with_permission = self.factory.makePerson() |
117 | + someone_else = self.factory.makePerson() |
118 | + branch = self.factory.makeAnyBranch() |
119 | + private_snap = self.factory.makeSnap( |
120 | + private=True, name="private-snap", |
121 | + owner=private_owner, registrant=private_owner, branch=branch) |
122 | + with person_logged_in(private_owner): |
123 | + private_snap.subscribe(user_with_permission, private_owner) |
124 | + self.factory.makeSnap( |
125 | + private=False, owner=private_owner, registrant=private_owner, |
126 | + branch=branch) |
127 | + full_list = """ |
128 | + Snap packages for lp:.* |
129 | + Name Owner Registered |
130 | + snap-name.* Random-user.* .* |
131 | + private-snap.* Random-user.* .*""" |
132 | + public_list = """ |
133 | + Snap packages for lp:.* |
134 | + Name Owner Registered |
135 | + snap-name.* Random-user.* .*""" |
136 | + |
137 | + self.assertTextMatchesExpressionIgnoreWhitespace( |
138 | + full_list, self.getMainText(branch, "+snaps", user=private_owner)) |
139 | + self.assertTextMatchesExpressionIgnoreWhitespace( |
140 | + full_list, self.getMainText(branch, "+snaps", |
141 | + user=user_with_permission)) |
142 | + self.assertTextMatchesExpressionIgnoreWhitespace( |
143 | + public_list, self.getMainText(branch, "+snaps", user=someone_else)) |
144 | + self.assertTextMatchesExpressionIgnoreWhitespace( |
145 | + public_list, self.getMainText(branch, "+snaps", user=None)) |
146 | + |
147 | + def test_git_repository_private_snap_listing(self): |
148 | + # Only certain users can see private snaps on git repo listing. |
149 | + self.useFixture(FeatureFixture(SNAP_TESTING_FLAGS)) |
150 | + private_owner = self.factory.makePerson(name="random-user") |
151 | + user_with_permission = self.factory.makePerson() |
152 | + someone_else = self.factory.makePerson() |
153 | + repository = self.factory.makeGitRepository() |
154 | + [ref] = self.factory.makeGitRefs(repository=repository) |
155 | + private_snap = self.factory.makeSnap( |
156 | + private=True, name="private-snap", |
157 | + owner=private_owner, registrant=private_owner, git_ref=ref) |
158 | + with person_logged_in(private_owner): |
159 | + private_snap.subscribe(user_with_permission, private_owner) |
160 | + self.factory.makeSnap( |
161 | + private=False, owner=private_owner, registrant=private_owner, |
162 | + git_ref=ref) |
163 | + |
164 | + full_list = """ |
165 | + Snap packages for lp:~.* |
166 | + Name Owner Registered |
167 | + snap-name.* Random-user.* .* |
168 | + private-snap.* Random-user.* .*""" |
169 | + public_list = """ |
170 | + Snap packages for lp:.* |
171 | + Name Owner Registered |
172 | + snap-name.* Random-user.* .*""" |
173 | + |
174 | + self.assertTextMatchesExpressionIgnoreWhitespace( |
175 | + full_list, |
176 | + self.getMainText(repository, "+snaps", user=private_owner)) |
177 | + self.assertTextMatchesExpressionIgnoreWhitespace( |
178 | + full_list, |
179 | + self.getMainText(repository, "+snaps", user=user_with_permission)) |
180 | + self.assertTextMatchesExpressionIgnoreWhitespace( |
181 | + public_list, |
182 | + self.getMainText(repository, "+snaps", user=someone_else)) |
183 | + self.assertTextMatchesExpressionIgnoreWhitespace( |
184 | + public_list, self.getMainText(repository, "+snaps", user=None)) |
185 | + |
186 | + def test_git_ref_private_snap_listing(self): |
187 | + # Only certain users can see private snaps on git ref listing. |
188 | + self.useFixture(FeatureFixture(SNAP_TESTING_FLAGS)) |
189 | + private_owner = self.factory.makePerson(name="random-user") |
190 | + user_with_permission = self.factory.makePerson() |
191 | + someone_else = self.factory.makePerson() |
192 | + repository = self.factory.makeGitRepository() |
193 | + [ref] = self.factory.makeGitRefs(repository=repository) |
194 | + private_snap = self.factory.makeSnap( |
195 | + private=True, name="private-snap", |
196 | + owner=private_owner, registrant=private_owner, git_ref=ref) |
197 | + with person_logged_in(private_owner): |
198 | + private_snap.subscribe(user_with_permission, private_owner) |
199 | + self.factory.makeSnap( |
200 | + private=False, owner=private_owner, registrant=private_owner, |
201 | + git_ref=ref) |
202 | + |
203 | + full_list = """ |
204 | + Snap packages for ~.*:.* |
205 | + Name Owner Registered |
206 | + snap-name.* Random-user.* .* |
207 | + private-snap.* Random-user.* .*""" |
208 | + public_list = """ |
209 | + Snap packages for ~.*:.* |
210 | + Name Owner Registered |
211 | + snap-name.* Random-user.* .*""" |
212 | + |
213 | + self.assertTextMatchesExpressionIgnoreWhitespace( |
214 | + full_list, self.getMainText(ref, "+snaps", user=private_owner)) |
215 | + self.assertTextMatchesExpressionIgnoreWhitespace( |
216 | + full_list, |
217 | + self.getMainText(ref, "+snaps", user=user_with_permission)) |
218 | + self.assertTextMatchesExpressionIgnoreWhitespace( |
219 | + public_list, self.getMainText(ref, "+snaps", user=someone_else)) |
220 | + self.assertTextMatchesExpressionIgnoreWhitespace( |
221 | + public_list, self.getMainText(ref, "+snaps", user=None)) |
222 | + |
223 | def assertSnapsQueryCount(self, context, item_creator): |
224 | self.pushConfig("launchpad", default_batch_size=10) |
225 | recorder1, recorder2 = record_two_runs( |
226 | diff --git a/lib/lp/snappy/model/snap.py b/lib/lp/snappy/model/snap.py |
227 | index 287ccc3..b101e18 100644 |
228 | --- a/lib/lp/snappy/model/snap.py |
229 | +++ b/lib/lp/snappy/model/snap.py |
230 | @@ -1438,7 +1438,8 @@ class SnapSet: |
231 | raise NoSuchSnap(name) |
232 | return snap |
233 | |
234 | - def _getSnapsFromCollection(self, collection, owner=None): |
235 | + def _getSnapsFromCollection(self, collection, owner=None, |
236 | + visible_by_user=None): |
237 | if IBranchCollection.providedBy(collection): |
238 | id_column = Snap.branch_id |
239 | ids = collection.getBranchIds() |
240 | @@ -1448,6 +1449,7 @@ class SnapSet: |
241 | expressions = [id_column.is_in(ids._get_select())] |
242 | if owner is not None: |
243 | expressions.append(Snap.owner == owner) |
244 | + expressions.append(get_snap_privacy_filter(visible_by_user)) |
245 | return IStore(Snap).find(Snap, *expressions) |
246 | |
247 | def findByIds(self, snap_ids, visible_by_user=None): |
248 | @@ -1465,8 +1467,10 @@ class SnapSet: |
249 | """See `ISnapSet`.""" |
250 | def _getSnaps(collection): |
251 | collection = collection.visibleByUser(visible_by_user) |
252 | - owned = self._getSnapsFromCollection(collection.ownedBy(person)) |
253 | - packaged = self._getSnapsFromCollection(collection, owner=person) |
254 | + owned = self._getSnapsFromCollection( |
255 | + collection.ownedBy(person), visible_by_user=visible_by_user) |
256 | + packaged = self._getSnapsFromCollection( |
257 | + collection, owner=person, visible_by_user=visible_by_user) |
258 | return owned.union(packaged) |
259 | |
260 | bzr_collection = removeSecurityProxy(getUtility(IAllBranches)) |
261 | @@ -1481,28 +1485,35 @@ class SnapSet: |
262 | """See `ISnapSet`.""" |
263 | def _getSnaps(collection): |
264 | return self._getSnapsFromCollection( |
265 | - collection.visibleByUser(visible_by_user)) |
266 | + collection.visibleByUser(visible_by_user), |
267 | + visible_by_user=visible_by_user) |
268 | |
269 | bzr_collection = removeSecurityProxy(IBranchCollection(project)) |
270 | git_collection = removeSecurityProxy(IGitCollection(project)) |
271 | return _getSnaps(bzr_collection).union(_getSnaps(git_collection)) |
272 | |
273 | - def findByBranch(self, branch): |
274 | + def findByBranch(self, branch, visible_by_user=None): |
275 | """See `ISnapSet`.""" |
276 | - return IStore(Snap).find(Snap, Snap.branch == branch) |
277 | + return IStore(Snap).find( |
278 | + Snap, |
279 | + Snap.branch == branch, |
280 | + get_snap_privacy_filter(visible_by_user)) |
281 | |
282 | - def findByGitRepository(self, repository, paths=None): |
283 | + def findByGitRepository(self, repository, paths=None, |
284 | + visible_by_user=None): |
285 | """See `ISnapSet`.""" |
286 | clauses = [Snap.git_repository == repository] |
287 | if paths is not None: |
288 | clauses.append(Snap.git_path.is_in(paths)) |
289 | + clauses.append(get_snap_privacy_filter(visible_by_user)) |
290 | return IStore(Snap).find(Snap, *clauses) |
291 | |
292 | - def findByGitRef(self, ref): |
293 | + def findByGitRef(self, ref, visible_by_user=None): |
294 | """See `ISnapSet`.""" |
295 | return IStore(Snap).find( |
296 | Snap, |
297 | - Snap.git_repository == ref.repository, Snap.git_path == ref.path) |
298 | + Snap.git_repository == ref.repository, Snap.git_path == ref.path, |
299 | + get_snap_privacy_filter(visible_by_user)) |
300 | |
301 | def findByContext(self, context, visible_by_user=None, order_by_date=True): |
302 | if IPerson.providedBy(context): |
303 | @@ -1510,16 +1521,13 @@ class SnapSet: |
304 | elif IProduct.providedBy(context): |
305 | snaps = self.findByProject( |
306 | context, visible_by_user=visible_by_user) |
307 | - # XXX cjwatson 2015-09-15: At the moment we can assume that if you |
308 | - # can see the source context then you can see the snap packages |
309 | - # based on it. This will cease to be true if snap packages gain |
310 | - # privacy of their own. |
311 | elif IBranch.providedBy(context): |
312 | - snaps = self.findByBranch(context) |
313 | + snaps = self.findByBranch(context, visible_by_user=visible_by_user) |
314 | elif IGitRepository.providedBy(context): |
315 | - snaps = self.findByGitRepository(context) |
316 | + snaps = self.findByGitRepository( |
317 | + context, visible_by_user=visible_by_user) |
318 | elif IGitRef.providedBy(context): |
319 | - snaps = self.findByGitRef(context) |
320 | + snaps = self.findByGitRef(context, visible_by_user=visible_by_user) |
321 | else: |
322 | raise BadSnapSearchContext(context) |
323 | if order_by_date: |