Merge ~pappacena/launchpad:snap-pillar-ui into launchpad:master
- Git
- lp:~pappacena/launchpad
- snap-pillar-ui
- Merge into master
Proposed by
Thiago F. Pappacena
Status: | Merged | ||||
---|---|---|---|---|---|
Approved by: | Thiago F. Pappacena | ||||
Approved revision: | 92b05132afbd563bf786c3204eacd2a96b10ab5b | ||||
Merge reported by: | Otto Co-Pilot | ||||
Merged at revision: | not available | ||||
Proposed branch: | ~pappacena/launchpad:snap-pillar-ui | ||||
Merge into: | launchpad:master | ||||
Prerequisite: | ~pappacena/launchpad:snap-pillar | ||||
Diff against target: |
751 lines (+275/-68) 7 files modified
lib/lp/registry/personmerge.py (+1/-1) lib/lp/snappy/browser/snap.py (+69/-15) lib/lp/snappy/browser/tests/test_snap.py (+84/-3) lib/lp/snappy/interfaces/snap.py (+16/-7) lib/lp/snappy/model/snap.py (+83/-31) lib/lp/snappy/tests/test_snap.py (+12/-6) lib/lp/testing/factory.py (+10/-5) |
||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Colin Watson (community) | Approve | ||
Review via email: mp+397529@code.launchpad.net |
Commit message
UI to associate a project pillar to Snaps on +admin view, and replacing "private" Snap flag with "information_type".
Description of the change
To post a comment you must log in.
Revision history for this message
Colin Watson (cjwatson) : | # |
review:
Needs Fixing
Revision history for this message
Thiago F. Pappacena (pappacena) wrote : | # |
Revision history for this message
Colin Watson (cjwatson) : | # |
review:
Approve
Revision history for this message
Thiago F. Pappacena (pappacena) : | # |
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | diff --git a/lib/lp/registry/personmerge.py b/lib/lp/registry/personmerge.py | |||
2 | index 50e0f91..949e8d9 100644 | |||
3 | --- a/lib/lp/registry/personmerge.py | |||
4 | +++ b/lib/lp/registry/personmerge.py | |||
5 | @@ -1,4 +1,4 @@ | |||
7 | 1 | # Copyright 2009-2018 Canonical Ltd. This software is licensed under the | 1 | # Copyright 2009-2021 Canonical Ltd. This software is licensed under the |
8 | 2 | # GNU Affero General Public License version 3 (see the file LICENSE). | 2 | # GNU Affero General Public License version 3 (see the file LICENSE). |
9 | 3 | 3 | ||
10 | 4 | """Person/team merger implementation.""" | 4 | """Person/team merger implementation.""" |
11 | diff --git a/lib/lp/snappy/browser/snap.py b/lib/lp/snappy/browser/snap.py | |||
12 | index 3b06a89..9759e66 100644 | |||
13 | --- a/lib/lp/snappy/browser/snap.py | |||
14 | +++ b/lib/lp/snappy/browser/snap.py | |||
15 | @@ -1,4 +1,4 @@ | |||
17 | 1 | # Copyright 2015-2020 Canonical Ltd. This software is licensed under the | 1 | # Copyright 2015-2021 Canonical Ltd. This software is licensed under the |
18 | 2 | # GNU Affero General Public License version 3 (see the file LICENSE). | 2 | # GNU Affero General Public License version 3 (see the file LICENSE). |
19 | 3 | 3 | ||
20 | 4 | """Snap views.""" | 4 | """Snap views.""" |
21 | @@ -48,10 +48,12 @@ from lp.app.browser.tales import format_link | |||
22 | 48 | from lp.app.enums import PRIVATE_INFORMATION_TYPES | 48 | from lp.app.enums import PRIVATE_INFORMATION_TYPES |
23 | 49 | from lp.app.interfaces.informationtype import IInformationType | 49 | from lp.app.interfaces.informationtype import IInformationType |
24 | 50 | from lp.app.interfaces.launchpad import ILaunchpadCelebrities | 50 | from lp.app.interfaces.launchpad import ILaunchpadCelebrities |
25 | 51 | from lp.app.vocabularies import InformationTypeVocabulary | ||
26 | 51 | from lp.app.widgets.itemswidgets import ( | 52 | from lp.app.widgets.itemswidgets import ( |
27 | 52 | LabeledMultiCheckBoxWidget, | 53 | LabeledMultiCheckBoxWidget, |
28 | 53 | LaunchpadDropdownWidget, | 54 | LaunchpadDropdownWidget, |
29 | 54 | LaunchpadRadioWidget, | 55 | LaunchpadRadioWidget, |
30 | 56 | LaunchpadRadioWidgetWithDescription, | ||
31 | 55 | ) | 57 | ) |
32 | 56 | from lp.buildmaster.interfaces.processor import IProcessorSet | 58 | from lp.buildmaster.interfaces.processor import IProcessorSet |
33 | 57 | from lp.code.browser.widgets.gitref import GitRefWidget | 59 | from lp.code.browser.widgets.gitref import GitRefWidget |
34 | @@ -343,7 +345,8 @@ class ISnapEditSchema(Interface): | |||
35 | 343 | use_template(ISnap, include=[ | 345 | use_template(ISnap, include=[ |
36 | 344 | 'owner', | 346 | 'owner', |
37 | 345 | 'name', | 347 | 'name', |
39 | 346 | 'private', | 348 | 'information_type', |
40 | 349 | 'project', | ||
41 | 347 | 'require_virtualized', | 350 | 'require_virtualized', |
42 | 348 | 'allow_internet', | 351 | 'allow_internet', |
43 | 349 | 'build_source_tarball', | 352 | 'build_source_tarball', |
44 | @@ -351,6 +354,7 @@ class ISnapEditSchema(Interface): | |||
45 | 351 | 'auto_build_channels', | 354 | 'auto_build_channels', |
46 | 352 | 'store_upload', | 355 | 'store_upload', |
47 | 353 | ]) | 356 | ]) |
48 | 357 | |||
49 | 354 | store_distro_series = Choice( | 358 | store_distro_series = Choice( |
50 | 355 | vocabulary='SnappyDistroSeries', required=True, | 359 | vocabulary='SnappyDistroSeries', required=True, |
51 | 356 | title='Series') | 360 | title='Series') |
52 | @@ -520,8 +524,12 @@ class SnapAddView( | |||
53 | 520 | kwargs = {'git_ref': self.context} | 524 | kwargs = {'git_ref': self.context} |
54 | 521 | else: | 525 | else: |
55 | 522 | kwargs = {'branch': self.context} | 526 | kwargs = {'branch': self.context} |
58 | 523 | private = not getUtility( | 527 | # XXX pappacena 2021-03-01: We should consider the pillar's branch |
59 | 524 | ISnapSet).isValidPrivacy(False, data['owner'], **kwargs) | 528 | # sharing policy when setting the information_type. |
60 | 529 | # Once we move the information_type and pillar edition from the | ||
61 | 530 | # admin view to the create/edit views, we should change this. | ||
62 | 531 | information_type = getUtility(ISnapSet).getSnapSuggestedPrivacy( | ||
63 | 532 | data['owner'], **kwargs) | ||
64 | 525 | if not data.get('auto_build', False): | 533 | if not data.get('auto_build', False): |
65 | 526 | data['auto_build_archive'] = None | 534 | data['auto_build_archive'] = None |
66 | 527 | data['auto_build_pocket'] = None | 535 | data['auto_build_pocket'] = None |
67 | @@ -532,7 +540,8 @@ class SnapAddView( | |||
68 | 532 | auto_build_archive=data['auto_build_archive'], | 540 | auto_build_archive=data['auto_build_archive'], |
69 | 533 | auto_build_pocket=data['auto_build_pocket'], | 541 | auto_build_pocket=data['auto_build_pocket'], |
70 | 534 | auto_build_channels=data['auto_build_channels'], | 542 | auto_build_channels=data['auto_build_channels'], |
72 | 535 | processors=data['processors'], private=private, | 543 | information_type=information_type, |
73 | 544 | processors=data['processors'], | ||
74 | 536 | build_source_tarball=data['build_source_tarball'], | 545 | build_source_tarball=data['build_source_tarball'], |
75 | 537 | store_upload=data['store_upload'], | 546 | store_upload=data['store_upload'], |
76 | 538 | store_series=data['store_distro_series'].snappy_series, | 547 | store_series=data['store_distro_series'].snappy_series, |
77 | @@ -559,6 +568,16 @@ class BaseSnapEditView(LaunchpadEditFormView, SnapAuthorizeMixin): | |||
78 | 559 | 568 | ||
79 | 560 | schema = ISnapEditSchema | 569 | schema = ISnapEditSchema |
80 | 561 | 570 | ||
81 | 571 | def getInformationTypesToShow(self): | ||
82 | 572 | """Get the information types to display on the edit form. | ||
83 | 573 | |||
84 | 574 | We display a customised set of information types: anything allowed | ||
85 | 575 | by the repository's model, plus the current type. | ||
86 | 576 | """ | ||
87 | 577 | allowed_types = set(self.context.getAllowedInformationTypes(self.user)) | ||
88 | 578 | allowed_types.add(self.context.information_type) | ||
89 | 579 | return allowed_types | ||
90 | 580 | |||
91 | 562 | @property | 581 | @property |
92 | 563 | def cancel_url(self): | 582 | def cancel_url(self): |
93 | 564 | return canonical_url(self.context) | 583 | return canonical_url(self.context) |
94 | @@ -612,25 +631,36 @@ class BaseSnapEditView(LaunchpadEditFormView, SnapAuthorizeMixin): | |||
95 | 612 | 631 | ||
96 | 613 | def validate(self, data): | 632 | def validate(self, data): |
97 | 614 | super(BaseSnapEditView, self).validate(data) | 633 | super(BaseSnapEditView, self).validate(data) |
100 | 615 | if data.get('private', self.context.private) is False: | 634 | info_type = data.get('information_type', self.context.information_type) |
101 | 616 | if 'private' in data or 'owner' in data: | 635 | editing_info_type = 'information_type' in data |
102 | 636 | private = info_type in PRIVATE_INFORMATION_TYPES | ||
103 | 637 | if private is False: | ||
104 | 638 | # These are the requirements for public snaps. | ||
105 | 639 | if 'information_type' in data or 'owner' in data: | ||
106 | 617 | owner = data.get('owner', self.context.owner) | 640 | owner = data.get('owner', self.context.owner) |
107 | 618 | if owner is not None and owner.private: | 641 | if owner is not None and owner.private: |
108 | 619 | self.setFieldError( | 642 | self.setFieldError( |
110 | 620 | 'private' if 'private' in data else 'owner', | 643 | 'information_type' if editing_info_type else 'owner', |
111 | 621 | 'A public snap cannot have a private owner.') | 644 | 'A public snap cannot have a private owner.') |
113 | 622 | if 'private' in data or 'branch' in data: | 645 | if 'information_type' in data or 'branch' in data: |
114 | 623 | branch = data.get('branch', self.context.branch) | 646 | branch = data.get('branch', self.context.branch) |
115 | 624 | if branch is not None and branch.private: | 647 | if branch is not None and branch.private: |
116 | 625 | self.setFieldError( | 648 | self.setFieldError( |
118 | 626 | 'private' if 'private' in data else 'branch', | 649 | 'information_type' if editing_info_type else 'branch', |
119 | 627 | 'A public snap cannot have a private branch.') | 650 | 'A public snap cannot have a private branch.') |
121 | 628 | if 'private' in data or 'git_ref' in data: | 651 | if 'information_type' in data or 'git_ref' in data: |
122 | 629 | ref = data.get('git_ref', self.context.git_ref) | 652 | ref = data.get('git_ref', self.context.git_ref) |
123 | 630 | if ref is not None and ref.private: | 653 | if ref is not None and ref.private: |
124 | 631 | self.setFieldError( | 654 | self.setFieldError( |
126 | 632 | 'private' if 'private' in data else 'git_ref', | 655 | 'information_type' if editing_info_type else 'git_ref', |
127 | 633 | 'A public snap cannot have a private repository.') | 656 | 'A public snap cannot have a private repository.') |
128 | 657 | else: | ||
129 | 658 | # Requirements for private snaps. | ||
130 | 659 | project = data.get('project', self.context.project) | ||
131 | 660 | if project is None: | ||
132 | 661 | msg = ('Private snap recipes must be associated ' | ||
133 | 662 | 'with a project.') | ||
134 | 663 | self.setFieldError('project', msg) | ||
135 | 634 | 664 | ||
136 | 635 | def _needStoreReauth(self, data): | 665 | def _needStoreReauth(self, data): |
137 | 636 | """Does this change require reauthorizing to the store?""" | 666 | """Does this change require reauthorizing to the store?""" |
138 | @@ -696,16 +726,40 @@ class SnapAdminView(BaseSnapEditView): | |||
139 | 696 | 726 | ||
140 | 697 | page_title = 'Administer' | 727 | page_title = 'Administer' |
141 | 698 | 728 | ||
143 | 699 | field_names = ['private', 'require_virtualized', 'allow_internet'] | 729 | # XXX pappacena 2021-02-19: Once we have the whole privacy work in |
144 | 730 | # place, we should move "project" and "information_type" from +admin | ||
145 | 731 | # page to +edit, to allow common users to edit this. | ||
146 | 732 | field_names = [ | ||
147 | 733 | 'project', 'information_type', 'require_virtualized', 'allow_internet'] | ||
148 | 734 | |||
149 | 735 | # See `setUpWidgets` method. | ||
150 | 736 | custom_widget_information_type = CustomWidgetFactory( | ||
151 | 737 | LaunchpadRadioWidgetWithDescription, | ||
152 | 738 | vocabulary=InformationTypeVocabulary(types=[])) | ||
153 | 739 | |||
154 | 740 | @property | ||
155 | 741 | def initial_values(self): | ||
156 | 742 | """Set initial values for the form.""" | ||
157 | 743 | # XXX pappacena 2021-02-12: Until we back fill information_type | ||
158 | 744 | # database column, it will be NULL, but snap.information_type | ||
159 | 745 | # property has a fallback to check "private" property. This should | ||
160 | 746 | # be removed once we back fill snap.information_type. | ||
161 | 747 | return {'information_type': self.context.information_type} | ||
162 | 748 | |||
163 | 749 | def setUpWidgets(self): | ||
164 | 750 | super(SnapAdminView, self).setUpWidgets() | ||
165 | 751 | info_type_widget = self.widgets['information_type'] | ||
166 | 752 | info_type_widget.vocabulary = InformationTypeVocabulary( | ||
167 | 753 | types=self.getInformationTypesToShow()) | ||
168 | 700 | 754 | ||
169 | 701 | def validate(self, data): | 755 | def validate(self, data): |
170 | 702 | super(SnapAdminView, self).validate(data) | 756 | super(SnapAdminView, self).validate(data) |
171 | 703 | # BaseSnapEditView.validate checks the rules for 'private' in | 757 | # BaseSnapEditView.validate checks the rules for 'private' in |
172 | 704 | # combination with other attributes. | 758 | # combination with other attributes. |
174 | 705 | if data.get('private', None) is True: | 759 | if data.get('information_type', None) in PRIVATE_INFORMATION_TYPES: |
175 | 706 | if not getFeatureFlag(SNAP_PRIVATE_FEATURE_FLAG): | 760 | if not getFeatureFlag(SNAP_PRIVATE_FEATURE_FLAG): |
176 | 707 | self.setFieldError( | 761 | self.setFieldError( |
178 | 708 | 'private', | 762 | 'information_type', |
179 | 709 | 'You do not have permission to create private snaps.') | 763 | 'You do not have permission to create private snaps.') |
180 | 710 | 764 | ||
181 | 711 | 765 | ||
182 | diff --git a/lib/lp/snappy/browser/tests/test_snap.py b/lib/lp/snappy/browser/tests/test_snap.py | |||
183 | index 3e48162..5f33900 100644 | |||
184 | --- a/lib/lp/snappy/browser/tests/test_snap.py | |||
185 | +++ b/lib/lp/snappy/browser/tests/test_snap.py | |||
186 | @@ -2,7 +2,7 @@ | |||
187 | 2 | # NOTE: The first line above must stay first; do not move the copyright | 2 | # NOTE: The first line above must stay first; do not move the copyright |
188 | 3 | # notice to the top. See http://www.python.org/dev/peps/pep-0263/. | 3 | # notice to the top. See http://www.python.org/dev/peps/pep-0263/. |
189 | 4 | # | 4 | # |
191 | 5 | # Copyright 2015-2020 Canonical Ltd. This software is licensed under the | 5 | # Copyright 2015-2021 Canonical Ltd. This software is licensed under the |
192 | 6 | # GNU Affero General Public License version 3 (see the file LICENSE). | 6 | # GNU Affero General Public License version 3 (see the file LICENSE). |
193 | 7 | 7 | ||
194 | 8 | """Test snap package views.""" | 8 | """Test snap package views.""" |
195 | @@ -85,6 +85,7 @@ from lp.testing import ( | |||
196 | 85 | admin_logged_in, | 85 | admin_logged_in, |
197 | 86 | BrowserTestCase, | 86 | BrowserTestCase, |
198 | 87 | login, | 87 | login, |
199 | 88 | login_admin, | ||
200 | 88 | login_person, | 89 | login_person, |
201 | 89 | person_logged_in, | 90 | person_logged_in, |
202 | 90 | TestCaseWithFactory, | 91 | TestCaseWithFactory, |
203 | @@ -400,6 +401,64 @@ class TestSnapAddView(BaseTestSnapView): | |||
204 | 400 | extract_text(find_tag_by_id(browser.contents, "privacy")) | 401 | extract_text(find_tag_by_id(browser.contents, "privacy")) |
205 | 401 | ) | 402 | ) |
206 | 402 | 403 | ||
207 | 404 | def test_create_new_snap_private_team_with_private_branch(self): | ||
208 | 405 | # Creating snaps from private branch should make the snap follow its | ||
209 | 406 | # privacy setting. | ||
210 | 407 | self.useFixture(BranchHostingFixture(blob=b"")) | ||
211 | 408 | login_person(self.person) | ||
212 | 409 | private_team = self.factory.makeTeam( | ||
213 | 410 | name='super-private', owner=self.person, | ||
214 | 411 | visibility=PersonVisibility.PRIVATE) | ||
215 | 412 | branch = self.factory.makeAnyBranch( | ||
216 | 413 | owner=self.person, registrant=self.person, | ||
217 | 414 | information_type=InformationType.PRIVATESECURITY) | ||
218 | 415 | |||
219 | 416 | browser = self.getViewBrowser( | ||
220 | 417 | branch, view_name="+new-snap", user=self.person) | ||
221 | 418 | browser.getControl(name="field.name").value = "private-snap" | ||
222 | 419 | browser.getControl("Owner").value = ['super-private'] | ||
223 | 420 | browser.getControl("Create snap package").click() | ||
224 | 421 | |||
225 | 422 | content = find_main_content(browser.contents) | ||
226 | 423 | self.assertEqual("private-snap", extract_text(content.h1)) | ||
227 | 424 | self.assertEqual( | ||
228 | 425 | 'This snap contains Private information', | ||
229 | 426 | extract_text(find_tag_by_id(browser.contents, "privacy")) | ||
230 | 427 | ) | ||
231 | 428 | login_admin() | ||
232 | 429 | snap = getUtility(ISnapSet).getByName(private_team, 'private-snap') | ||
233 | 430 | self.assertEqual( | ||
234 | 431 | InformationType.PRIVATESECURITY, snap.information_type) | ||
235 | 432 | |||
236 | 433 | def test_create_new_snap_private_team_with_private_git_repo(self): | ||
237 | 434 | # Creating snaps from private repos should make the snap follow its | ||
238 | 435 | # privacy setting. | ||
239 | 436 | self.useFixture(BranchHostingFixture(blob=b"")) | ||
240 | 437 | login_person(self.person) | ||
241 | 438 | private_team = self.factory.makeTeam( | ||
242 | 439 | name='super-private', owner=self.person, | ||
243 | 440 | visibility=PersonVisibility.PRIVATE) | ||
244 | 441 | [git_ref] = self.factory.makeGitRefs( | ||
245 | 442 | owner=self.person, registrant=self.person, | ||
246 | 443 | information_type=InformationType.PRIVATESECURITY) | ||
247 | 444 | |||
248 | 445 | browser = self.getViewBrowser( | ||
249 | 446 | git_ref, view_name="+new-snap", user=self.person) | ||
250 | 447 | browser.getControl(name="field.name").value = "private-snap" | ||
251 | 448 | browser.getControl("Owner").value = ['super-private'] | ||
252 | 449 | browser.getControl("Create snap package").click() | ||
253 | 450 | |||
254 | 451 | content = find_main_content(browser.contents) | ||
255 | 452 | self.assertEqual("private-snap", extract_text(content.h1)) | ||
256 | 453 | self.assertEqual( | ||
257 | 454 | 'This snap contains Private information', | ||
258 | 455 | extract_text(find_tag_by_id(browser.contents, "privacy")) | ||
259 | 456 | ) | ||
260 | 457 | login_admin() | ||
261 | 458 | snap = getUtility(ISnapSet).getByName(private_team, 'private-snap') | ||
262 | 459 | self.assertEqual( | ||
263 | 460 | InformationType.PRIVATESECURITY, snap.information_type) | ||
264 | 461 | |||
265 | 403 | def test_create_new_snap_build_source_tarball(self): | 462 | def test_create_new_snap_build_source_tarball(self): |
266 | 404 | # We can create a new snap and ask for it to build a source tarball. | 463 | # We can create a new snap and ask for it to build a source tarball. |
267 | 405 | self.useFixture(BranchHostingFixture(blob=b"")) | 464 | self.useFixture(BranchHostingFixture(blob=b"")) |
268 | @@ -655,22 +714,43 @@ class TestSnapAdminView(BaseTestSnapView): | |||
269 | 655 | member_of=[getUtility(ILaunchpadCelebrities).commercial_admin]) | 714 | member_of=[getUtility(ILaunchpadCelebrities).commercial_admin]) |
270 | 656 | login_person(self.person) | 715 | login_person(self.person) |
271 | 657 | snap = self.factory.makeSnap(registrant=self.person) | 716 | snap = self.factory.makeSnap(registrant=self.person) |
272 | 717 | project = self.factory.makeProduct(name='my-project') | ||
273 | 658 | self.assertTrue(snap.require_virtualized) | 718 | self.assertTrue(snap.require_virtualized) |
274 | 719 | self.assertIsNone(snap.project) | ||
275 | 659 | self.assertFalse(snap.private) | 720 | self.assertFalse(snap.private) |
276 | 660 | self.assertTrue(snap.allow_internet) | 721 | self.assertTrue(snap.allow_internet) |
277 | 661 | 722 | ||
278 | 723 | private = InformationType.PRIVATESECURITY.name | ||
279 | 662 | browser = self.getViewBrowser(snap, user=commercial_admin) | 724 | browser = self.getViewBrowser(snap, user=commercial_admin) |
280 | 663 | browser.getLink("Administer snap package").click() | 725 | browser.getLink("Administer snap package").click() |
281 | 726 | browser.getControl(name='field.project').value = "my-project" | ||
282 | 664 | browser.getControl("Require virtualized builders").selected = False | 727 | browser.getControl("Require virtualized builders").selected = False |
284 | 665 | browser.getControl("Private").selected = True | 728 | browser.getControl(name="field.information_type").value = private |
285 | 666 | browser.getControl("Allow external network access").selected = False | 729 | browser.getControl("Allow external network access").selected = False |
286 | 667 | browser.getControl("Update snap package").click() | 730 | browser.getControl("Update snap package").click() |
287 | 668 | 731 | ||
288 | 669 | login_person(self.person) | 732 | login_person(self.person) |
289 | 733 | self.assertEqual(project, snap.project) | ||
290 | 670 | self.assertFalse(snap.require_virtualized) | 734 | self.assertFalse(snap.require_virtualized) |
291 | 671 | self.assertTrue(snap.private) | 735 | self.assertTrue(snap.private) |
292 | 672 | self.assertFalse(snap.allow_internet) | 736 | self.assertFalse(snap.allow_internet) |
293 | 673 | 737 | ||
294 | 738 | def test_admin_snap_private_without_project(self): | ||
295 | 739 | # Cannot make snap private if it doesn't have a project associated. | ||
296 | 740 | login_person(self.person) | ||
297 | 741 | snap = self.factory.makeSnap(registrant=self.person) | ||
298 | 742 | commercial_admin = self.factory.makePerson( | ||
299 | 743 | member_of=[getUtility(ILaunchpadCelebrities).commercial_admin]) | ||
300 | 744 | private = InformationType.PRIVATESECURITY.name | ||
301 | 745 | browser = self.getViewBrowser(snap, user=commercial_admin) | ||
302 | 746 | browser.getLink("Administer snap package").click() | ||
303 | 747 | browser.getControl(name='field.project').value = '' | ||
304 | 748 | browser.getControl(name="field.information_type").value = private | ||
305 | 749 | browser.getControl("Update snap package").click() | ||
306 | 750 | self.assertEqual( | ||
307 | 751 | 'Private snap recipes must be associated with a project.', | ||
308 | 752 | extract_text(find_tags_by_class(browser.contents, "message")[1])) | ||
309 | 753 | |||
310 | 674 | def test_admin_snap_privacy_mismatch(self): | 754 | def test_admin_snap_privacy_mismatch(self): |
311 | 675 | # Cannot make snap public if it still contains private information. | 755 | # Cannot make snap public if it still contains private information. |
312 | 676 | login_person(self.person) | 756 | login_person(self.person) |
313 | @@ -682,9 +762,10 @@ class TestSnapAdminView(BaseTestSnapView): | |||
314 | 682 | # can reach this snap because it's owned by a private team. | 762 | # can reach this snap because it's owned by a private team. |
315 | 683 | commercial_admin = self.factory.makePerson( | 763 | commercial_admin = self.factory.makePerson( |
316 | 684 | member_of=[getUtility(ILaunchpadCelebrities).commercial_admin]) | 764 | member_of=[getUtility(ILaunchpadCelebrities).commercial_admin]) |
317 | 765 | public = InformationType.PUBLIC.name | ||
318 | 685 | browser = self.getViewBrowser(snap, user=commercial_admin) | 766 | browser = self.getViewBrowser(snap, user=commercial_admin) |
319 | 686 | browser.getLink("Administer snap package").click() | 767 | browser.getLink("Administer snap package").click() |
321 | 687 | browser.getControl("Private").selected = False | 768 | browser.getControl(name="field.information_type").value = public |
322 | 688 | browser.getControl("Update snap package").click() | 769 | browser.getControl("Update snap package").click() |
323 | 689 | self.assertEqual( | 770 | self.assertEqual( |
324 | 690 | 'A public snap cannot have a private owner.', | 771 | 'A public snap cannot have a private owner.', |
325 | diff --git a/lib/lp/snappy/interfaces/snap.py b/lib/lp/snappy/interfaces/snap.py | |||
326 | index 9b19b1b..8ac5838 100644 | |||
327 | --- a/lib/lp/snappy/interfaces/snap.py | |||
328 | +++ b/lib/lp/snappy/interfaces/snap.py | |||
329 | @@ -67,6 +67,7 @@ from lazr.restful.fields import ( | |||
330 | 67 | Reference, | 67 | Reference, |
331 | 68 | ReferenceChoice, | 68 | ReferenceChoice, |
332 | 69 | ) | 69 | ) |
333 | 70 | from lazr.restful.interface import copy_field | ||
334 | 70 | from six.moves import http_client | 71 | from six.moves import http_client |
335 | 71 | from zope.interface import ( | 72 | from zope.interface import ( |
336 | 72 | Attribute, | 73 | Attribute, |
337 | @@ -570,6 +571,12 @@ class ISnapView(Interface): | |||
338 | 570 | # Really ISnapBuild, patched in lp.snappy.interfaces.webservice. | 571 | # Really ISnapBuild, patched in lp.snappy.interfaces.webservice. |
339 | 571 | value_type=Reference(schema=Interface), readonly=True))) | 572 | value_type=Reference(schema=Interface), readonly=True))) |
340 | 572 | 573 | ||
341 | 574 | def getAllowedInformationTypes(user): | ||
342 | 575 | """Get a list of acceptable `InformationType`s for this snap recipe. | ||
343 | 576 | |||
344 | 577 | If the user is a Launchpad admin, any type is acceptable. | ||
345 | 578 | """ | ||
346 | 579 | |||
347 | 573 | 580 | ||
348 | 574 | class ISnapEdit(IWebhookTarget): | 581 | class ISnapEdit(IWebhookTarget): |
349 | 575 | """`ISnap` methods that require launchpad.Edit permission.""" | 582 | """`ISnap` methods that require launchpad.Edit permission.""" |
350 | @@ -672,7 +679,7 @@ class ISnapEditableAttributes(IHasOwner): | |||
351 | 672 | description=_("The owner of this snap package."))) | 679 | description=_("The owner of this snap package."))) |
352 | 673 | 680 | ||
353 | 674 | project = ReferenceChoice( | 681 | project = ReferenceChoice( |
355 | 675 | title=_('The project that this Snap is associated with.'), | 682 | title=_('The project that this Snap is associated with'), |
356 | 676 | schema=IProduct, vocabulary='Product', | 683 | schema=IProduct, vocabulary='Product', |
357 | 677 | required=False, readonly=False) | 684 | required=False, readonly=False) |
358 | 678 | 685 | ||
359 | @@ -845,7 +852,7 @@ class ISnapAdminAttributes(Interface): | |||
360 | 845 | 852 | ||
361 | 846 | information_type = exported(Choice( | 853 | information_type = exported(Choice( |
362 | 847 | title=_("Information type"), vocabulary=InformationType, | 854 | title=_("Information type"), vocabulary=InformationType, |
364 | 848 | required=True, readonly=True, default=InformationType.PUBLIC, | 855 | required=True, readonly=False, default=InformationType.PUBLIC, |
365 | 849 | description=_( | 856 | description=_( |
366 | 850 | "The type of information contained in this Snap recipe."))) | 857 | "The type of information contained in this Snap recipe."))) |
367 | 851 | 858 | ||
368 | @@ -884,6 +891,7 @@ class ISnapSet(Interface): | |||
369 | 884 | 891 | ||
370 | 885 | @call_with(registrant=REQUEST_USER) | 892 | @call_with(registrant=REQUEST_USER) |
371 | 886 | @operation_parameters( | 893 | @operation_parameters( |
372 | 894 | information_type=copy_field(ISnap["information_type"], required=False), | ||
373 | 887 | processors=List( | 895 | processors=List( |
374 | 888 | value_type=Reference(schema=IProcessor), required=False)) | 896 | value_type=Reference(schema=IProcessor), required=False)) |
375 | 889 | @export_factory_operation( | 897 | @export_factory_operation( |
376 | @@ -891,15 +899,16 @@ class ISnapSet(Interface): | |||
377 | 891 | "owner", "distro_series", "name", "description", "branch", | 899 | "owner", "distro_series", "name", "description", "branch", |
378 | 892 | "git_repository", "git_repository_url", "git_path", "git_ref", | 900 | "git_repository", "git_repository_url", "git_path", "git_ref", |
379 | 893 | "auto_build", "auto_build_archive", "auto_build_pocket", | 901 | "auto_build", "auto_build_archive", "auto_build_pocket", |
382 | 894 | "private", "store_upload", "store_series", "store_name", | 902 | "store_upload", "store_series", "store_name", "store_channels", |
383 | 895 | "store_channels"]) | 903 | "project"]) |
384 | 896 | @operation_for_version("devel") | 904 | @operation_for_version("devel") |
385 | 897 | def new(registrant, owner, distro_series, name, description=None, | 905 | def new(registrant, owner, distro_series, name, description=None, |
386 | 898 | branch=None, git_repository=None, git_repository_url=None, | 906 | branch=None, git_repository=None, git_repository_url=None, |
387 | 899 | git_path=None, git_ref=None, auto_build=False, | 907 | git_path=None, git_ref=None, auto_build=False, |
388 | 900 | auto_build_archive=None, auto_build_pocket=None, | 908 | auto_build_archive=None, auto_build_pocket=None, |
389 | 901 | require_virtualized=True, processors=None, date_created=None, | 909 | require_virtualized=True, processors=None, date_created=None, |
391 | 902 | private=False, store_upload=False, store_series=None, | 910 | information_type=InformationType.PUBLIC, store_upload=False, |
392 | 911 | store_series=None, | ||
393 | 903 | store_name=None, store_secrets=None, store_channels=None, | 912 | store_name=None, store_secrets=None, store_channels=None, |
394 | 904 | project=None): | 913 | project=None): |
395 | 905 | """Create an `ISnap`.""" | 914 | """Create an `ISnap`.""" |
396 | @@ -907,8 +916,8 @@ class ISnapSet(Interface): | |||
397 | 907 | def exists(owner, name): | 916 | def exists(owner, name): |
398 | 908 | """Check to see if a matching snap exists.""" | 917 | """Check to see if a matching snap exists.""" |
399 | 909 | 918 | ||
402 | 910 | def isValidPrivacy(private, owner, branch=None, git_ref=None): | 919 | def getSnapSuggestedPrivacy(owner, branch=None, git_ref=None): |
403 | 911 | """Whether or not the privacy context is valid.""" | 920 | """Which privacy a Snap should have based on its creation params.""" |
404 | 912 | 921 | ||
405 | 913 | def isValidInformationType( | 922 | def isValidInformationType( |
406 | 914 | information_type, owner, branch=None, git_ref=None): | 923 | information_type, owner, branch=None, git_ref=None): |
407 | diff --git a/lib/lp/snappy/model/snap.py b/lib/lp/snappy/model/snap.py | |||
408 | index 8ed9215..2c298fb 100644 | |||
409 | --- a/lib/lp/snappy/model/snap.py | |||
410 | +++ b/lib/lp/snappy/model/snap.py | |||
411 | @@ -29,6 +29,7 @@ from storm.expr import ( | |||
412 | 29 | Not, | 29 | Not, |
413 | 30 | Or, | 30 | Or, |
414 | 31 | Select, | 31 | Select, |
415 | 32 | SQL, | ||
416 | 32 | ) | 33 | ) |
417 | 33 | from storm.locals import ( | 34 | from storm.locals import ( |
418 | 34 | Bool, | 35 | Bool, |
419 | @@ -58,11 +59,14 @@ from lp.app.browser.tales import ( | |||
420 | 58 | DateTimeFormatterAPI, | 59 | DateTimeFormatterAPI, |
421 | 59 | ) | 60 | ) |
422 | 60 | from lp.app.enums import ( | 61 | from lp.app.enums import ( |
423 | 62 | FREE_INFORMATION_TYPES, | ||
424 | 61 | InformationType, | 63 | InformationType, |
425 | 62 | PRIVATE_INFORMATION_TYPES, | 64 | PRIVATE_INFORMATION_TYPES, |
426 | 65 | PUBLIC_INFORMATION_TYPES, | ||
427 | 63 | ) | 66 | ) |
428 | 64 | from lp.app.errors import IncompatibleArguments | 67 | from lp.app.errors import IncompatibleArguments |
429 | 65 | from lp.app.interfaces.security import IAuthorization | 68 | from lp.app.interfaces.security import IAuthorization |
430 | 69 | from lp.app.interfaces.services import IService | ||
431 | 66 | from lp.buildmaster.enums import BuildStatus | 70 | from lp.buildmaster.enums import BuildStatus |
432 | 67 | from lp.buildmaster.interfaces.buildqueue import IBuildQueueSet | 71 | from lp.buildmaster.interfaces.buildqueue import IBuildQueueSet |
433 | 68 | from lp.buildmaster.model.builder import Builder | 72 | from lp.buildmaster.model.builder import Builder |
434 | @@ -95,6 +99,10 @@ from lp.code.interfaces.gitrepository import ( | |||
435 | 95 | ) | 99 | ) |
436 | 96 | from lp.code.model.branch import Branch | 100 | from lp.code.model.branch import Branch |
437 | 97 | from lp.code.model.branchcollection import GenericBranchCollection | 101 | from lp.code.model.branchcollection import GenericBranchCollection |
438 | 102 | from lp.code.model.branchnamespace import ( | ||
439 | 103 | BRANCH_POLICY_ALLOWED_TYPES, | ||
440 | 104 | BRANCH_POLICY_REQUIRED_GRANTS, | ||
441 | 105 | ) | ||
442 | 98 | from lp.code.model.gitcollection import GenericGitCollection | 106 | from lp.code.model.gitcollection import GenericGitCollection |
443 | 99 | from lp.code.model.gitrepository import GitRepository | 107 | from lp.code.model.gitrepository import GitRepository |
444 | 100 | from lp.registry.errors import PrivatePersonLinkageError | 108 | from lp.registry.errors import PrivatePersonLinkageError |
445 | @@ -202,6 +210,17 @@ def snap_modified(snap, event): | |||
446 | 202 | removeSecurityProxy(snap).date_last_modified = UTC_NOW | 210 | removeSecurityProxy(snap).date_last_modified = UTC_NOW |
447 | 203 | 211 | ||
448 | 204 | 212 | ||
449 | 213 | def user_has_special_snap_access(user): | ||
450 | 214 | """Admins have special access. | ||
451 | 215 | |||
452 | 216 | :param user: An `IPerson` or None. | ||
453 | 217 | """ | ||
454 | 218 | if user is None: | ||
455 | 219 | return False | ||
456 | 220 | roles = IPersonRoles(user) | ||
457 | 221 | return roles.in_admin | ||
458 | 222 | |||
459 | 223 | |||
460 | 205 | @implementer(ISnapBuildRequest) | 224 | @implementer(ISnapBuildRequest) |
461 | 206 | class SnapBuildRequest: | 225 | class SnapBuildRequest: |
462 | 207 | """See `ISnapBuildRequest`. | 226 | """See `ISnapBuildRequest`. |
463 | @@ -351,13 +370,7 @@ class Snap(Storm, WebhookTargetMixin): | |||
464 | 351 | 370 | ||
465 | 352 | require_virtualized = Bool(name='require_virtualized') | 371 | require_virtualized = Bool(name='require_virtualized') |
466 | 353 | 372 | ||
474 | 354 | def _validate_private(self, attr, value): | 373 | _private = Bool(name='private') |
468 | 355 | if not getUtility(ISnapSet).isValidPrivacy( | ||
469 | 356 | value, self.owner, self.branch, self.git_ref): | ||
470 | 357 | raise SnapPrivacyMismatch | ||
471 | 358 | return value | ||
472 | 359 | |||
473 | 360 | private = Bool(name='private', validator=_validate_private) | ||
475 | 361 | 374 | ||
476 | 362 | def _valid_information_type(self, attr, value): | 375 | def _valid_information_type(self, attr, value): |
477 | 363 | if not getUtility(ISnapSet).isValidInformationType( | 376 | if not getUtility(ISnapSet).isValidInformationType( |
478 | @@ -389,20 +402,18 @@ class Snap(Storm, WebhookTargetMixin): | |||
479 | 389 | description=None, branch=None, git_ref=None, auto_build=False, | 402 | description=None, branch=None, git_ref=None, auto_build=False, |
480 | 390 | auto_build_archive=None, auto_build_pocket=None, | 403 | auto_build_archive=None, auto_build_pocket=None, |
481 | 391 | auto_build_channels=None, require_virtualized=True, | 404 | auto_build_channels=None, require_virtualized=True, |
486 | 392 | date_created=DEFAULT, private=False, allow_internet=True, | 405 | date_created=DEFAULT, information_type=InformationType.PUBLIC, |
487 | 393 | build_source_tarball=False, store_upload=False, | 406 | allow_internet=True, build_source_tarball=False, |
488 | 394 | store_series=None, store_name=None, store_secrets=None, | 407 | store_upload=False, store_series=None, store_name=None, |
489 | 395 | store_channels=None, project=None): | 408 | store_secrets=None, store_channels=None, project=None): |
490 | 396 | """Construct a `Snap`.""" | 409 | """Construct a `Snap`.""" |
491 | 397 | super(Snap, self).__init__() | 410 | super(Snap, self).__init__() |
492 | 398 | 411 | ||
494 | 399 | # Set the private flag first so that other validators can perform | 412 | # Set the information type first so that other validators can perform |
495 | 400 | # suitable privacy checks, but pillar should also be set, since it's | 413 | # suitable privacy checks, but pillar should also be set, since it's |
496 | 401 | # mandatory for private snaps. | 414 | # mandatory for private snaps. |
497 | 402 | self.project = project | 415 | self.project = project |
501 | 403 | self.private = private | 416 | self.information_type = information_type |
499 | 404 | self.information_type = (InformationType.PROPRIETARY if private else | ||
500 | 405 | InformationType.PUBLIC) | ||
502 | 406 | 417 | ||
503 | 407 | self.registrant = registrant | 418 | self.registrant = registrant |
504 | 408 | self.owner = owner | 419 | self.owner = owner |
505 | @@ -432,7 +443,7 @@ class Snap(Storm, WebhookTargetMixin): | |||
506 | 432 | @property | 443 | @property |
507 | 433 | def information_type(self): | 444 | def information_type(self): |
508 | 434 | if self._information_type is None: | 445 | if self._information_type is None: |
510 | 435 | return (InformationType.PROPRIETARY if self.private | 446 | return (InformationType.PROPRIETARY if self._private |
511 | 436 | else InformationType.PUBLIC) | 447 | else InformationType.PUBLIC) |
512 | 437 | return self._information_type | 448 | return self._information_type |
513 | 438 | 449 | ||
514 | @@ -441,6 +452,10 @@ class Snap(Storm, WebhookTargetMixin): | |||
515 | 441 | self._information_type = information_type | 452 | self._information_type = information_type |
516 | 442 | 453 | ||
517 | 443 | @property | 454 | @property |
518 | 455 | def private(self): | ||
519 | 456 | return self.information_type not in PUBLIC_INFORMATION_TYPES | ||
520 | 457 | |||
521 | 458 | @property | ||
522 | 444 | def valid_webhook_event_types(self): | 459 | def valid_webhook_event_types(self): |
523 | 445 | return ["snap:build:0.1"] | 460 | return ["snap:build:0.1"] |
524 | 446 | 461 | ||
525 | @@ -618,6 +633,24 @@ class Snap(Storm, WebhookTargetMixin): | |||
526 | 618 | def store_channels(self, value): | 633 | def store_channels(self, value): |
527 | 619 | self._store_channels = value or None | 634 | self._store_channels = value or None |
528 | 620 | 635 | ||
529 | 636 | def getAllowedInformationTypes(self, user): | ||
530 | 637 | """See `ISnap`.""" | ||
531 | 638 | if user_has_special_snap_access(user): | ||
532 | 639 | # Admins can set any type. | ||
533 | 640 | return set(PUBLIC_INFORMATION_TYPES + PRIVATE_INFORMATION_TYPES) | ||
534 | 641 | if self.pillar is None: | ||
535 | 642 | return set(FREE_INFORMATION_TYPES) | ||
536 | 643 | required_grant = BRANCH_POLICY_REQUIRED_GRANTS[ | ||
537 | 644 | self.project.branch_sharing_policy] | ||
538 | 645 | if (required_grant is not None | ||
539 | 646 | and not getUtility(IService, 'sharing').checkPillarAccess( | ||
540 | 647 | [self.project], required_grant, self.owner) | ||
541 | 648 | and (user is None | ||
542 | 649 | or not getUtility(IService, 'sharing').checkPillarAccess( | ||
543 | 650 | [self.project], required_grant, user))): | ||
544 | 651 | return [] | ||
545 | 652 | return BRANCH_POLICY_ALLOWED_TYPES[self.project.branch_sharing_policy] | ||
546 | 653 | |||
547 | 621 | @staticmethod | 654 | @staticmethod |
548 | 622 | def extractSSOCaveats(macaroon): | 655 | def extractSSOCaveats(macaroon): |
549 | 623 | locations = [ | 656 | locations = [ |
550 | @@ -1143,10 +1176,11 @@ class SnapSet: | |||
551 | 1143 | git_path=None, git_ref=None, auto_build=False, | 1176 | git_path=None, git_ref=None, auto_build=False, |
552 | 1144 | auto_build_archive=None, auto_build_pocket=None, | 1177 | auto_build_archive=None, auto_build_pocket=None, |
553 | 1145 | auto_build_channels=None, require_virtualized=True, | 1178 | auto_build_channels=None, require_virtualized=True, |
558 | 1146 | processors=None, date_created=DEFAULT, private=False, | 1179 | processors=None, date_created=DEFAULT, |
559 | 1147 | allow_internet=True, build_source_tarball=False, | 1180 | information_type=InformationType.PUBLIC, allow_internet=True, |
560 | 1148 | store_upload=False, store_series=None, store_name=None, | 1181 | build_source_tarball=False, store_upload=False, |
561 | 1149 | store_secrets=None, store_channels=None, project=None): | 1182 | store_series=None, store_name=None, store_secrets=None, |
562 | 1183 | store_channels=None, project=None): | ||
563 | 1150 | """See `ISnapSet`.""" | 1184 | """See `ISnapSet`.""" |
564 | 1151 | if not registrant.inTeam(owner): | 1185 | if not registrant.inTeam(owner): |
565 | 1152 | if owner.is_team: | 1186 | if owner.is_team: |
566 | @@ -1183,7 +1217,8 @@ class SnapSet: | |||
567 | 1183 | # IntegrityError due to exceptions being raised during object | 1217 | # IntegrityError due to exceptions being raised during object |
568 | 1184 | # creation and to ensure that everything relevant is in the Storm | 1218 | # creation and to ensure that everything relevant is in the Storm |
569 | 1185 | # cache. | 1219 | # cache. |
571 | 1186 | if not self.isValidPrivacy(private, owner, branch, git_ref): | 1220 | if not self.isValidInformationType( |
572 | 1221 | information_type, owner, branch, git_ref): | ||
573 | 1187 | raise SnapPrivacyMismatch | 1222 | raise SnapPrivacyMismatch |
574 | 1188 | 1223 | ||
575 | 1189 | store = IMasterStore(Snap) | 1224 | store = IMasterStore(Snap) |
576 | @@ -1194,7 +1229,7 @@ class SnapSet: | |||
577 | 1194 | auto_build_pocket=auto_build_pocket, | 1229 | auto_build_pocket=auto_build_pocket, |
578 | 1195 | auto_build_channels=auto_build_channels, | 1230 | auto_build_channels=auto_build_channels, |
579 | 1196 | require_virtualized=require_virtualized, date_created=date_created, | 1231 | require_virtualized=require_virtualized, date_created=date_created, |
581 | 1197 | private=private, allow_internet=allow_internet, | 1232 | information_type=information_type, allow_internet=allow_internet, |
582 | 1198 | build_source_tarball=build_source_tarball, | 1233 | build_source_tarball=build_source_tarball, |
583 | 1199 | store_upload=store_upload, store_series=store_series, | 1234 | store_upload=store_upload, store_series=store_series, |
584 | 1200 | store_name=store_name, store_secrets=store_secrets, | 1235 | store_name=store_name, store_secrets=store_secrets, |
585 | @@ -1208,9 +1243,24 @@ class SnapSet: | |||
586 | 1208 | 1243 | ||
587 | 1209 | return snap | 1244 | return snap |
588 | 1210 | 1245 | ||
590 | 1211 | def isValidPrivacy(self, private, owner, branch=None, git_ref=None): | 1246 | def getSnapSuggestedPrivacy(self, owner, branch=None, git_ref=None): |
591 | 1212 | """See `ISnapSet`.""" | 1247 | """See `ISnapSet`.""" |
593 | 1213 | # Private snaps may contain anything ... | 1248 | # Public snaps with private sources are not allowed. |
594 | 1249 | source = branch or git_ref | ||
595 | 1250 | if source is not None and source.private: | ||
596 | 1251 | return source.information_type | ||
597 | 1252 | |||
598 | 1253 | # Public snaps owned by private teams are not allowed. | ||
599 | 1254 | if owner is not None and owner.private: | ||
600 | 1255 | return InformationType.PROPRIETARY | ||
601 | 1256 | |||
602 | 1257 | # XXX pappacena 2021-03-02: We need to consider the pillar's branch | ||
603 | 1258 | # sharing policy here instead of suggesting PUBLIC. | ||
604 | 1259 | return InformationType.PUBLIC | ||
605 | 1260 | |||
606 | 1261 | def isValidInformationType(self, information_type, owner, branch=None, | ||
607 | 1262 | git_ref=None): | ||
608 | 1263 | private = information_type not in PUBLIC_INFORMATION_TYPES | ||
609 | 1214 | if private: | 1264 | if private: |
610 | 1215 | # If appropriately enabled via feature flag. | 1265 | # If appropriately enabled via feature flag. |
611 | 1216 | if not getFeatureFlag(SNAP_PRIVATE_FEATURE_FLAG): | 1266 | if not getFeatureFlag(SNAP_PRIVATE_FEATURE_FLAG): |
612 | @@ -1228,11 +1278,6 @@ class SnapSet: | |||
613 | 1228 | 1278 | ||
614 | 1229 | return True | 1279 | return True |
615 | 1230 | 1280 | ||
616 | 1231 | def isValidInformationType(self, information_type, owner, branch=None, | ||
617 | 1232 | git_ref=None): | ||
618 | 1233 | private = information_type in PRIVATE_INFORMATION_TYPES | ||
619 | 1234 | return self.isValidPrivacy(private, owner, branch, git_ref) | ||
620 | 1235 | |||
621 | 1236 | def _getByName(self, owner, name): | 1281 | def _getByName(self, owner, name): |
622 | 1237 | return IStore(Snap).find( | 1282 | return IStore(Snap).find( |
623 | 1238 | Snap, Snap.owner == owner, Snap.name == name).one() | 1283 | Snap, Snap.owner == owner, Snap.name == name).one() |
624 | @@ -1333,15 +1378,22 @@ class SnapSet: | |||
625 | 1333 | # XXX cjwatson 2016-11-25: This is in principle a poor query, but we | 1378 | # XXX cjwatson 2016-11-25: This is in principle a poor query, but we |
626 | 1334 | # don't yet have the access grant infrastructure to do better, and | 1379 | # don't yet have the access grant infrastructure to do better, and |
627 | 1335 | # in any case the numbers involved should be very small. | 1380 | # in any case the numbers involved should be very small. |
628 | 1381 | # XXX pappacena 2021-02-12: Once we do the migration to back fill | ||
629 | 1382 | # information_type, we should be able to change this. | ||
630 | 1383 | private_snap = SQL( | ||
631 | 1384 | "CASE information_type" | ||
632 | 1385 | " WHEN NULL THEN private" | ||
633 | 1386 | " ELSE information_type NOT IN ?" | ||
634 | 1387 | "END", params=[tuple(i.value for i in PUBLIC_INFORMATION_TYPES)]) | ||
635 | 1336 | if visible_by_user is None: | 1388 | if visible_by_user is None: |
637 | 1337 | return Snap.private == False | 1389 | return private_snap == False |
638 | 1338 | else: | 1390 | else: |
639 | 1339 | roles = IPersonRoles(visible_by_user) | 1391 | roles = IPersonRoles(visible_by_user) |
640 | 1340 | if roles.in_admin or roles.in_commercial_admin: | 1392 | if roles.in_admin or roles.in_commercial_admin: |
641 | 1341 | return True | 1393 | return True |
642 | 1342 | else: | 1394 | else: |
643 | 1343 | return Or( | 1395 | return Or( |
645 | 1344 | Snap.private == False, | 1396 | private_snap == False, |
646 | 1345 | Snap.owner_id.is_in(Select( | 1397 | Snap.owner_id.is_in(Select( |
647 | 1346 | TeamParticipation.teamID, | 1398 | TeamParticipation.teamID, |
648 | 1347 | TeamParticipation.person == visible_by_user))) | 1399 | TeamParticipation.person == visible_by_user))) |
649 | diff --git a/lib/lp/snappy/tests/test_snap.py b/lib/lp/snappy/tests/test_snap.py | |||
650 | index cdf8813..755851a 100644 | |||
651 | --- a/lib/lp/snappy/tests/test_snap.py | |||
652 | +++ b/lib/lp/snappy/tests/test_snap.py | |||
653 | @@ -166,7 +166,8 @@ class TestSnapFeatureFlag(TestCaseWithFactory): | |||
654 | 166 | self.assertRaises( | 166 | self.assertRaises( |
655 | 167 | SnapPrivateFeatureDisabled, getUtility(ISnapSet).new, | 167 | SnapPrivateFeatureDisabled, getUtility(ISnapSet).new, |
656 | 168 | person, person, None, None, | 168 | person, person, None, None, |
658 | 169 | branch=self.factory.makeAnyBranch(), private=True) | 169 | branch=self.factory.makeAnyBranch(), |
659 | 170 | information_type=InformationType.PROPRIETARY) | ||
660 | 170 | 171 | ||
661 | 171 | 172 | ||
662 | 172 | class TestSnap(TestCaseWithFactory): | 173 | class TestSnap(TestCaseWithFactory): |
663 | @@ -1419,13 +1420,15 @@ class TestSnapSet(TestCaseWithFactory): | |||
664 | 1419 | 1420 | ||
665 | 1420 | def test_private_snap_information_type_compatibility(self): | 1421 | def test_private_snap_information_type_compatibility(self): |
666 | 1421 | login_admin() | 1422 | login_admin() |
667 | 1423 | private = InformationType.PROPRIETARY | ||
668 | 1424 | public = InformationType.PUBLIC | ||
669 | 1422 | private_snap = getUtility(ISnapSet).new( | 1425 | private_snap = getUtility(ISnapSet).new( |
671 | 1423 | private=True, **self.makeSnapComponents()) | 1426 | information_type=private, **self.makeSnapComponents()) |
672 | 1424 | self.assertEqual( | 1427 | self.assertEqual( |
673 | 1425 | InformationType.PROPRIETARY, private_snap.information_type) | 1428 | InformationType.PROPRIETARY, private_snap.information_type) |
674 | 1426 | 1429 | ||
675 | 1427 | public_snap = getUtility(ISnapSet).new( | 1430 | public_snap = getUtility(ISnapSet).new( |
677 | 1428 | private=False, **self.makeSnapComponents()) | 1431 | information_type=public, **self.makeSnapComponents()) |
678 | 1429 | self.assertEqual( | 1432 | self.assertEqual( |
679 | 1430 | InformationType.PUBLIC, public_snap.information_type) | 1433 | InformationType.PUBLIC, public_snap.information_type) |
680 | 1431 | 1434 | ||
681 | @@ -1433,7 +1436,7 @@ class TestSnapSet(TestCaseWithFactory): | |||
682 | 1433 | # Creating private snaps for public sources is allowed. | 1436 | # Creating private snaps for public sources is allowed. |
683 | 1434 | [ref] = self.factory.makeGitRefs() | 1437 | [ref] = self.factory.makeGitRefs() |
684 | 1435 | components = self.makeSnapComponents(git_ref=ref) | 1438 | components = self.makeSnapComponents(git_ref=ref) |
686 | 1436 | components['private'] = True | 1439 | components['information_type'] = InformationType.PROPRIETARY |
687 | 1437 | components['project'] = self.factory.makeProduct() | 1440 | components['project'] = self.factory.makeProduct() |
688 | 1438 | snap = getUtility(ISnapSet).new(**components) | 1441 | snap = getUtility(ISnapSet).new(**components) |
689 | 1439 | with person_logged_in(components['owner']): | 1442 | with person_logged_in(components['owner']): |
690 | @@ -2519,9 +2522,11 @@ class TestSnapWebservice(TestCaseWithFactory): | |||
691 | 2519 | if auto_build_pocket is not None: | 2522 | if auto_build_pocket is not None: |
692 | 2520 | kwargs["auto_build_pocket"] = auto_build_pocket.title | 2523 | kwargs["auto_build_pocket"] = auto_build_pocket.title |
693 | 2521 | logout() | 2524 | logout() |
694 | 2525 | information_type = (InformationType.PROPRIETARY if private else | ||
695 | 2526 | InformationType.PUBLIC) | ||
696 | 2522 | response = webservice.named_post( | 2527 | response = webservice.named_post( |
697 | 2523 | "/+snaps", "new", owner=owner_url, distro_series=distroseries_url, | 2528 | "/+snaps", "new", owner=owner_url, distro_series=distroseries_url, |
699 | 2524 | name="mir", private=private, **kwargs) | 2529 | name="mir", information_type=information_type.title, **kwargs) |
700 | 2525 | self.assertEqual(201, response.status) | 2530 | self.assertEqual(201, response.status) |
701 | 2526 | return webservice.get(response.getHeader("Location")).jsonBody() | 2531 | return webservice.get(response.getHeader("Location")).jsonBody() |
702 | 2527 | 2532 | ||
703 | @@ -2666,7 +2671,8 @@ class TestSnapWebservice(TestCaseWithFactory): | |||
704 | 2666 | admin, permission=OAuthPermission.WRITE_PRIVATE) | 2671 | admin, permission=OAuthPermission.WRITE_PRIVATE) |
705 | 2667 | admin_webservice.default_api_version = "devel" | 2672 | admin_webservice.default_api_version = "devel" |
706 | 2668 | response = admin_webservice.patch( | 2673 | response = admin_webservice.patch( |
708 | 2669 | snap_url, "application/json", json.dumps({"private": False})) | 2674 | snap_url, "application/json", |
709 | 2675 | json.dumps({"information_type": 'Public'})) | ||
710 | 2670 | self.assertEqual(400, response.status) | 2676 | self.assertEqual(400, response.status) |
711 | 2671 | self.assertEqual( | 2677 | self.assertEqual( |
712 | 2672 | b"Snap recipe contains private information and cannot be public.", | 2678 | b"Snap recipe contains private information and cannot be public.", |
713 | diff --git a/lib/lp/testing/factory.py b/lib/lp/testing/factory.py | |||
714 | index 9712bea..bc89da6 100644 | |||
715 | --- a/lib/lp/testing/factory.py | |||
716 | +++ b/lib/lp/testing/factory.py | |||
717 | @@ -4748,10 +4748,10 @@ class BareLaunchpadObjectFactory(ObjectFactory): | |||
718 | 4748 | auto_build_archive=None, auto_build_pocket=None, | 4748 | auto_build_archive=None, auto_build_pocket=None, |
719 | 4749 | auto_build_channels=None, is_stale=None, | 4749 | auto_build_channels=None, is_stale=None, |
720 | 4750 | require_virtualized=True, processors=None, | 4750 | require_virtualized=True, processors=None, |
725 | 4751 | date_created=DEFAULT, private=False, allow_internet=True, | 4751 | date_created=DEFAULT, private=False, information_type=None, |
726 | 4752 | build_source_tarball=False, store_upload=False, | 4752 | allow_internet=True, build_source_tarball=False, |
727 | 4753 | store_series=None, store_name=None, store_secrets=None, | 4753 | store_upload=False, store_series=None, store_name=None, |
728 | 4754 | store_channels=None, project=_DEFAULT): | 4754 | store_secrets=None, store_channels=None, project=_DEFAULT): |
729 | 4755 | """Make a new Snap.""" | 4755 | """Make a new Snap.""" |
730 | 4756 | if registrant is None: | 4756 | if registrant is None: |
731 | 4757 | registrant = self.makePerson() | 4757 | registrant = self.makePerson() |
732 | @@ -4775,13 +4775,18 @@ class BareLaunchpadObjectFactory(ObjectFactory): | |||
733 | 4775 | project = self.makeProduct() | 4775 | project = self.makeProduct() |
734 | 4776 | if project is _DEFAULT: | 4776 | if project is _DEFAULT: |
735 | 4777 | project = None | 4777 | project = None |
736 | 4778 | assert information_type is None or private is None | ||
737 | 4779 | if private is not None: | ||
738 | 4780 | information_type = (InformationType.PUBLIC if not private | ||
739 | 4781 | else InformationType.PROPRIETARY) | ||
740 | 4778 | snap = getUtility(ISnapSet).new( | 4782 | snap = getUtility(ISnapSet).new( |
741 | 4779 | registrant, owner, distroseries, name, | 4783 | registrant, owner, distroseries, name, |
742 | 4780 | require_virtualized=require_virtualized, processors=processors, | 4784 | require_virtualized=require_virtualized, processors=processors, |
743 | 4781 | date_created=date_created, branch=branch, git_ref=git_ref, | 4785 | date_created=date_created, branch=branch, git_ref=git_ref, |
744 | 4782 | auto_build=auto_build, auto_build_archive=auto_build_archive, | 4786 | auto_build=auto_build, auto_build_archive=auto_build_archive, |
745 | 4783 | auto_build_pocket=auto_build_pocket, | 4787 | auto_build_pocket=auto_build_pocket, |
747 | 4784 | auto_build_channels=auto_build_channels, private=private, | 4788 | auto_build_channels=auto_build_channels, |
748 | 4789 | information_type=information_type, | ||
749 | 4785 | allow_internet=allow_internet, | 4790 | allow_internet=allow_internet, |
750 | 4786 | build_source_tarball=build_source_tarball, | 4791 | build_source_tarball=build_source_tarball, |
751 | 4787 | store_upload=store_upload, store_series=store_series, | 4792 | store_upload=store_upload, store_series=store_series, |
Pushed the requested changes. This should be ready for another round of review.