Launchpad itself

Merge lp:~bac/launchpad/bug-588773-charm into lp:launchpad

  1. bug-588773-charm
  2. Merge into devel
Proposed by Brad Crittenden
Status: Merged
Approved by: Edwin Grubbs
Approved revision: no longer in the source branch.
Merged at revision: 11411
Proposed branch: lp:~bac/launchpad/bug-588773-charm
Merge into: lp:launchpad
Diff against target: 372 lines (+145/-32)
8 files modified
lib/lp/registry/browser/person.py (+12/-1)
lib/lp/registry/browser/product.py (+16/-3)
lib/lp/registry/browser/project.py (+11/-4)
lib/lp/registry/configure.zcml (+18/-6)
lib/lp/registry/interfaces/projectgroup.py (+14/-9)
lib/lp/registry/stories/person/xx-admin-person-review.txt (+16/-3)
lib/lp/registry/stories/product/xx-product-edit.txt (+33/-5)
lib/lp/registry/stories/project/xx-project-edit.txt (+25/-1)
To merge this branch: bzr merge lp:~bac/launchpad/bug-588773-charm
Reviewer Review Type Date Requested Status
Edwin Grubbs (community) code Approve
Brad Crittenden Pending
Review via email: mp+33248@code.launchpad.net

Commit message

Fix the permissions for pillar editing by registry admins.

Description of the change

= Summary =

A second stab at bug 588773 landed this week. It correctly gave
registry experts access to the necessary +review/+admin pages but,
unfortunately, the permissions for some individual attributes was not
fixed. The test walked up to the pages but didn't make the changes and
submit them, so the failure wasn't seen until QA.

== Proposed fix ==

Change the permissions on the required fields. Some fields have been
removed from the page the registry experts see. Those fields are
editable by the object owner, so there is no need to involve registry in
them.

== Pre-implementation notes ==

None.

== Implementation details ==

As above

== Tests ==

bin/test -vv -t (xx-project-edit.txt | xx-product-edit.txt | \
   xx-admin-person-review.txt)

In reality, all registry tests should be run:

bin/test -vvm lp.registry

== Demo and Q/A ==

http://launchpad.dev/firefox and follow 'Administer' and submit changes.
http://launchpad.dev/mozilla and follow 'Administer' and submit changes.
http://launchpad.dev/~name12 and follow 'Administer' and submit changes.
http://launchpad.dev/ubuntu/hoary and follow 'Administer' and submit
changes.

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/registry/browser/product.py
  lib/lp/registry/configure.zcml
  lib/lp/registry/stories/project/xx-project-edit.txt
  lib/lp/registry/stories/product/xx-product-edit.txt
  lib/lp/registry/stories/person/xx-admin-person-review.txt
  lib/lp/registry/interfaces/projectgroup.py
  lib/lp/registry/browser/project.py

To post a comment you must log in.
Revision history for this message
Brad Crittenden (bac) wrote :

Hi Edwin,

The problem with personal standing you mentioned on IRC has been fixed:
http://pastebin.ubuntu.com/481141

The UI problem you mentioned is an artifact of the way menus work/are broken. Curtis and I discussed it and came up with a solution: http://pastebin.ubuntu.com/481143/

He and I think that fix should be done in a separate branch dedicated to fixing menus as it may have unintended consequences. So, I ask that you not consider the UI issue a flaw of this branch.

Revision history for this message
Edwin Grubbs (edwin-grubbs) wrote :

Hi Brad,

The changes look good. It's fine to fix the menu in a different branch.

-Edwin

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/registry/browser/person.py'
2--- lib/lp/registry/browser/person.py 2010-08-20 20:31:18 +0000
3+++ lib/lp/registry/browser/person.py 2010-08-22 17:51:44 +0000
4@@ -1692,12 +1692,23 @@
5 """Administer an `IPerson`."""
6 schema = IPerson
7 label = "Review person"
8- field_names = [
9+ default_field_names = [
10 'name', 'displayname',
11 'personal_standing', 'personal_standing_reason']
12 custom_widget(
13 'personal_standing_reason', TextAreaWidget, height=5, width=60)
14
15+ def setUpFields(self):
16+ """Setup the normal fields from the schema, as appropriate.
17+
18+ If not an admin (e.g. registry expert), remove the displayname field.
19+ """
20+ self.field_names = self.default_field_names[:]
21+ admin = check_permission('launchpad.Admin', self.context)
22+ if not admin:
23+ self.field_names.remove('displayname')
24+ super(PersonAdministerView, self).setUpFields()
25+
26 @property
27 def is_viewing_person(self):
28 """Is the view showing an `IPerson`?
29
30=== modified file 'lib/lp/registry/browser/product.py'
31--- lib/lp/registry/browser/product.py 2010-08-20 20:31:18 +0000
32+++ lib/lp/registry/browser/product.py 2010-08-22 17:51:44 +0000
33@@ -1535,7 +1535,13 @@
34 class ProductAdminView(ProductEditView, ProductValidationMixin):
35 """View for $project/+admin"""
36 label = "Administer project details"
37- field_names = ["name", "owner", "active", "autoupdate", "private_bugs"]
38+ default_field_names = [
39+ "name",
40+ "owner",
41+ "active",
42+ "autoupdate",
43+ "private_bugs",
44+ ]
45
46 @property
47 def page_title(self):
48@@ -1549,9 +1555,16 @@
49 proper widget created by default. Even though it is read-only, admins
50 need the ability to change it.
51 """
52+ self.field_names = self.default_field_names[:]
53+ admin = check_permission('launchpad.Admin', self.context)
54+ if not admin:
55+ self.field_names.remove('owner')
56+ self.field_names.remove('autoupdate')
57 super(ProductAdminView, self).setUpFields()
58- self.form_fields = (self._createAliasesField() + self.form_fields
59- + self._createRegistrantField())
60+ self.form_fields = self._createAliasesField() + self.form_fields
61+ if admin:
62+ self.form_fields = (
63+ self.form_fields + self._createRegistrantField())
64
65 def _createAliasesField(self):
66 """Return a PillarAliases field for IProduct.aliases."""
67
68=== modified file 'lib/lp/registry/browser/project.py'
69--- lib/lp/registry/browser/project.py 2010-08-20 20:31:18 +0000
70+++ lib/lp/registry/browser/project.py 2010-08-22 17:51:44 +0000
71@@ -61,6 +61,7 @@
72 stepthrough,
73 structured,
74 )
75+from canonical.launchpad.webapp.authorization import check_permission
76 from canonical.launchpad.webapp.breadcrumb import Breadcrumb
77 from canonical.launchpad.webapp.menu import NavigationMenu
78 from lp.answers.browser.question import QuestionAddView
79@@ -355,7 +356,6 @@
80 'homepageurl', 'bugtracker', 'sourceforgeproject',
81 'freshmeatproject', 'wikiurl']
82
83-
84 @action('Change Details', name='change')
85 def edit(self, action, data):
86 self.updateContextFromData(data)
87@@ -373,7 +373,7 @@
88 class ProjectReviewView(ProjectEditView):
89
90 label = "Review upstream project group details"
91- field_names = ['name', 'owner', 'active', 'reviewed']
92+ default_field_names = ['name', 'owner', 'active', 'reviewed']
93
94 def setUpFields(self):
95 """Setup the normal fields from the schema plus adds 'Registrant'.
96@@ -382,9 +382,16 @@
97 proper widget created by default. Even though it is read-only, admins
98 need the ability to change it.
99 """
100+ self.field_names = self.default_field_names[:]
101+ admin = check_permission('launchpad.Admin', self.context)
102+ if not admin:
103+ self.field_names.remove('name')
104+ self.field_names.remove('owner')
105 super(ProjectReviewView, self).setUpFields()
106- self.form_fields = (self._createAliasesField() + self.form_fields
107- + self._createRegistrantField())
108+ self.form_fields = self._createAliasesField() + self.form_fields
109+ if admin:
110+ self.form_fields = (
111+ self.form_fields + self._createRegistrantField())
112
113 def _createAliasesField(self):
114 """Return a PillarAliases field for IProjectGroup.aliases."""
115
116=== modified file 'lib/lp/registry/configure.zcml'
117--- lib/lp/registry/configure.zcml 2010-08-19 03:26:03 +0000
118+++ lib/lp/registry/configure.zcml 2010-08-22 17:51:44 +0000
119@@ -313,9 +313,16 @@
120 attributes="
121 aliases"/>
122 <require
123- permission="launchpad.Admin"
124+ permission="launchpad.Moderate"
125 attributes="
126 setAliases"/>
127+
128+ <!-- IProjectGroupModerate -->
129+ <allow
130+ interface="lp.registry.interfaces.projectgroup.IProjectGroupModerate"/>
131+ <require
132+ permission="launchpad.Moderate"
133+ set_schema="lp.registry.interfaces.projectgroup.IProjectGroupModerate"/>
134 </class>
135 <adapter
136 for="lp.registry.interfaces.projectgroup.IProjectGroup"
137@@ -825,8 +832,12 @@
138 interface="lp.registry.interfaces.person.IPersonEditRestricted"/>
139 <require
140 permission="launchpad.Edit"
141- set_schema="lp.registry.interfaces.person.IPersonPublic lp.registry.interfaces.person.ITeamPublic"
142- set_attributes="name displayname"/>
143+ set_schema="lp.registry.interfaces.person.IPersonPublic
144+ lp.registry.interfaces.person.ITeamPublic"
145+ set_attributes="displayname"/>
146+ <require
147+ permission="launchpad.Moderate"
148+ set_attributes="name"/>
149 <require
150 permission="launchpad.Moderate"
151 interface="lp.registry.interfaces.person.IPersonModerate" />
152@@ -837,7 +848,7 @@
153 permission="launchpad.Special"
154 interface="lp.registry.interfaces.person.IPersonSpecialRestricted"/>
155 <require
156- permission="launchpad.Edit"
157+ permission="launchpad.Moderate"
158 set_schema="lp.registry.interfaces.person.IHasStanding"/>
159 </class>
160
161@@ -1117,8 +1128,9 @@
162 <!-- https://lists.ubuntu.com/mailman/private/launchpad/2007-April/015189.html
163 for further discussion - stub 20070411 -->
164
165+ <!-- Per bug 588773, changing to launchpad.Moderate to allow Registry Experts (~registry) -->
166 <require
167- permission="launchpad.Admin"
168+ permission="launchpad.Moderate"
169 set_attributes="name autoupdate registrant"/>
170 <require
171 permission="launchpad.TranslationsAdmin"
172@@ -1153,7 +1165,7 @@
173 attributes="
174 aliases"/>
175 <require
176- permission="launchpad.Admin"
177+ permission="launchpad.Moderate"
178 attributes="
179 setAliases"/>
180
181
182=== modified file 'lib/lp/registry/interfaces/projectgroup.py'
183--- lib/lp/registry/interfaces/projectgroup.py 2010-08-20 20:31:18 +0000
184+++ lib/lp/registry/interfaces/projectgroup.py 2010-08-22 17:51:44 +0000
185@@ -97,12 +97,21 @@
186 return IProjectGroup
187
188
189+class IProjectGroupModerate(IPillar):
190+ """IProjectGroup attributes used with launchpad.Moderate permission."""
191+ reviewed = exported(
192+ Bool(
193+ title=_('Reviewed'), required=False,
194+ description=_("Whether or not this project group has been "
195+ "reviewed.")))
196+
197+
198 class IProjectGroupPublic(
199 ICanGetMilestonesDirectly, IHasAppointedDriver, IHasBranches, IHasBugs,
200 IHasDrivers, IHasBranchVisibilityPolicy, IHasIcon, IHasLogo,
201 IHasMentoringOffers, IHasMergeProposals, IHasMilestones, IHasMugshot,
202 IHasOwner, IHasSpecifications, IHasSprints, IMakesAnnouncements,
203- IKarmaContext, IPillar, IRootContext, IHasOfficialBugTags):
204+ IKarmaContext, IRootContext, IHasOfficialBugTags):
205 """Public IProjectGroup properties."""
206
207 id = Int(title=_('ID'), readonly=True)
208@@ -267,12 +276,6 @@
209 "displayed on this project group's home page in Launchpad. "
210 "It should be no bigger than 100kb in size. ")))
211
212- reviewed = exported(
213- Bool(
214- title=_('Reviewed'), required=False,
215- description=_("Whether or not this project group has been "
216- "reviewed.")))
217-
218 bugtracker = exported(
219 ReferenceChoice(title=_('Bug Tracker'), required=False,
220 vocabulary='BugTracker', schema=IBugTracker,
221@@ -326,8 +329,10 @@
222 """Return a ProjectGroupSeries object with name `series_name`."""
223
224
225-class IProjectGroup(IProjectGroupPublic, IStructuralSubscriptionTarget,
226- ITranslationPolicy):
227+class IProjectGroup(IProjectGroupPublic,
228+ IProjectGroupModerate,
229+ IStructuralSubscriptionTarget,
230+ ITranslationPolicy):
231 """A ProjectGroup."""
232
233 export_as_webservice_entry('project_group')
234
235=== modified file 'lib/lp/registry/stories/person/xx-admin-person-review.txt'
236--- lib/lp/registry/stories/person/xx-admin-person-review.txt 2010-08-16 19:10:11 +0000
237+++ lib/lp/registry/stories/person/xx-admin-person-review.txt 2010-08-22 17:51:44 +0000
238@@ -94,8 +94,7 @@
239 Registry experts
240 ----------------
241
242-Registry experts can see the +review page in full, just like Launchpad
243-admins.
244+Registry experts can see the +review page, minus the displayname.
245
246 >>> email = "expert@example.com"
247 >>> password = "test"
248@@ -106,10 +105,24 @@
249 >>> print expert_browser.title
250 Review person...
251
252+ >>> expert_browser.getControl('Name', index=0).value = "no-way"
253+ >>> expert_browser.getControl(
254+ ... 'Display Name', index=0)
255+ Traceback (most recent call last):
256+ ...
257+ LookupError: label 'Display Name'
258+
259+ >>> expert_browser.getControl('Personal standing').value = ['GOOD']
260+ >>> expert_browser.getControl(
261+ ... name='field.personal_standing_reason').value = 'good guy'
262+ >>> expert_browser.getControl('Change').click()
263+ >>> print expert_browser.url
264+ http://launchpad.dev/~no-way
265+
266 However, members of the registry team only get partial access to the
267 review account page to be able to suspend spam accounts.
268
269- >>> expert_browser.open('http://launchpad.dev/~no-priv/+reviewaccount')
270+ >>> expert_browser.open('http://launchpad.dev/~no-way/+reviewaccount')
271 >>> print expert_browser.title
272 Review person's account...
273
274
275=== modified file 'lib/lp/registry/stories/product/xx-product-edit.txt'
276--- lib/lp/registry/stories/product/xx-product-edit.txt 2010-08-13 21:30:24 +0000
277+++ lib/lp/registry/stories/product/xx-product-edit.txt 2010-08-22 17:51:44 +0000
278@@ -235,22 +235,50 @@
279 ...
280 Unauthorized:...
281
282-Tf we add them to the Registry Experts team:
283+If we add them to the Registry Experts team:
284
285 >>> admin_browser.open("http://launchpad.dev/~registry/+addmember")
286 >>> admin_browser.getControl('New member').value = 'no-priv'
287 >>> admin_browser.getControl('Add Member').click()
288
289-They cannot edit projects.
290+They still cannot edit projects.
291
292 >>> browser.open('http://launchpad.dev/firefox/+edit')
293 Traceback (most recent call last):
294 ...
295 Unauthorized:...
296
297-But they still can access +admin.
298-
299- >>> browser.open('http://launchpad.dev/firefox/+admin')
300+But they can access +admin, though it is more restricted than that for admins.
301+
302+ >>> from lp.testing import login, logout
303+ >>> login('admin@canonical.com')
304+ >>> product = factory.makeProduct(name='trebuche')
305+ >>> logout()
306+
307+The registry experts do not have access to the maintainer or
308+registrant fields.
309+
310+ >>> browser.open('http://launchpad.dev/trebuche/+admin')
311+ >>> browser.getControl('Maintainer')
312+ Traceback (most recent call last):
313+ ...
314+ LookupError: label 'Maintainer'
315+ >>> browser.getControl('Registrant')
316+ Traceback (most recent call last):
317+ ...
318+ LookupError: label 'Registrant'
319+
320+But registry experts can change a product name and set an alias.
321+
322+ >>> browser.getControl('Name').value = 'trebuchet'
323+ >>> browser.getControl('Aliases').value = 'trebucket'
324+ >>> browser.getControl('Change').click()
325+
326+ >>> browser.getLink('Administer').click()
327+ >>> print browser.getControl('Name').value
328+ trebuchet
329+ >>> print browser.getControl('Aliases').value
330+ trebucket
331
332
333 Display error when trying to remove all licenses
334
335=== modified file 'lib/lp/registry/stories/project/xx-project-edit.txt'
336--- lib/lp/registry/stories/project/xx-project-edit.txt 2010-08-16 19:10:11 +0000
337+++ lib/lp/registry/stories/project/xx-project-edit.txt 2010-08-22 17:51:44 +0000
338@@ -132,9 +132,33 @@
339 ...
340 Unauthorized...
341
342-Registry experts do have full access to administer project groups.
343+Registry experts do have access to administer project groups, though
344+there are fewer fields available.
345
346 >>> expert_browser.open('http://launchpad.dev/new-name')
347 >>> expert_browser.getLink('Administer').click()
348 >>> print expert_browser.url
349 http://launchpad.dev/new-name/+review
350+
351+ >>> expert_browser.getControl('Maintainer')
352+ Traceback (most recent call last):
353+ ...
354+ LookupError: label 'Maintainer'
355+ >>> expert_browser.getControl('Registrant')
356+ Traceback (most recent call last):
357+ ...
358+ LookupError: label 'Registrant'
359+ >>> expert_browser.getControl('Name')
360+ Traceback (most recent call last):
361+ ...
362+ LookupError: label 'Name'
363+
364+ >>> expert_browser.getControl('Aliases').value = 'sleepy'
365+ >>> expert_browser.getControl('Active').selected = False
366+ >>> expert_browser.getControl('Reviewed').selected = False
367+ >>> expert_browser.getControl('Change').click()
368+
369+ >>> expert_browser.open('http://launchpad.dev/new-name')
370+ >>> expert_browser.getLink('Administer').click()
371+ >>> print expert_browser.getControl('Aliases').value
372+ sleepy