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
=== modified file 'lib/lp/registry/browser/person.py'
--- lib/lp/registry/browser/person.py 2010-08-20 20:31:18 +0000
+++ lib/lp/registry/browser/person.py 2010-08-22 17:51:44 +0000
@@ -1692,12 +1692,23 @@
1692 """Administer an `IPerson`."""1692 """Administer an `IPerson`."""
1693 schema = IPerson1693 schema = IPerson
1694 label = "Review person"1694 label = "Review person"
1695 field_names = [1695 default_field_names = [
1696 'name', 'displayname',1696 'name', 'displayname',
1697 'personal_standing', 'personal_standing_reason']1697 'personal_standing', 'personal_standing_reason']
1698 custom_widget(1698 custom_widget(
1699 'personal_standing_reason', TextAreaWidget, height=5, width=60)1699 'personal_standing_reason', TextAreaWidget, height=5, width=60)
17001700
1701 def setUpFields(self):
1702 """Setup the normal fields from the schema, as appropriate.
1703
1704 If not an admin (e.g. registry expert), remove the displayname field.
1705 """
1706 self.field_names = self.default_field_names[:]
1707 admin = check_permission('launchpad.Admin', self.context)
1708 if not admin:
1709 self.field_names.remove('displayname')
1710 super(PersonAdministerView, self).setUpFields()
1711
1701 @property1712 @property
1702 def is_viewing_person(self):1713 def is_viewing_person(self):
1703 """Is the view showing an `IPerson`?1714 """Is the view showing an `IPerson`?
17041715
=== modified file 'lib/lp/registry/browser/product.py'
--- lib/lp/registry/browser/product.py 2010-08-20 20:31:18 +0000
+++ lib/lp/registry/browser/product.py 2010-08-22 17:51:44 +0000
@@ -1535,7 +1535,13 @@
1535class ProductAdminView(ProductEditView, ProductValidationMixin):1535class ProductAdminView(ProductEditView, ProductValidationMixin):
1536 """View for $project/+admin"""1536 """View for $project/+admin"""
1537 label = "Administer project details"1537 label = "Administer project details"
1538 field_names = ["name", "owner", "active", "autoupdate", "private_bugs"]1538 default_field_names = [
1539 "name",
1540 "owner",
1541 "active",
1542 "autoupdate",
1543 "private_bugs",
1544 ]
15391545
1540 @property1546 @property
1541 def page_title(self):1547 def page_title(self):
@@ -1549,9 +1555,16 @@
1549 proper widget created by default. Even though it is read-only, admins1555 proper widget created by default. Even though it is read-only, admins
1550 need the ability to change it.1556 need the ability to change it.
1551 """1557 """
1558 self.field_names = self.default_field_names[:]
1559 admin = check_permission('launchpad.Admin', self.context)
1560 if not admin:
1561 self.field_names.remove('owner')
1562 self.field_names.remove('autoupdate')
1552 super(ProductAdminView, self).setUpFields()1563 super(ProductAdminView, self).setUpFields()
1553 self.form_fields = (self._createAliasesField() + self.form_fields1564 self.form_fields = self._createAliasesField() + self.form_fields
1554 + self._createRegistrantField())1565 if admin:
1566 self.form_fields = (
1567 self.form_fields + self._createRegistrantField())
15551568
1556 def _createAliasesField(self):1569 def _createAliasesField(self):
1557 """Return a PillarAliases field for IProduct.aliases."""1570 """Return a PillarAliases field for IProduct.aliases."""
15581571
=== modified file 'lib/lp/registry/browser/project.py'
--- lib/lp/registry/browser/project.py 2010-08-20 20:31:18 +0000
+++ lib/lp/registry/browser/project.py 2010-08-22 17:51:44 +0000
@@ -61,6 +61,7 @@
61 stepthrough,61 stepthrough,
62 structured,62 structured,
63 )63 )
64from canonical.launchpad.webapp.authorization import check_permission
64from canonical.launchpad.webapp.breadcrumb import Breadcrumb65from canonical.launchpad.webapp.breadcrumb import Breadcrumb
65from canonical.launchpad.webapp.menu import NavigationMenu66from canonical.launchpad.webapp.menu import NavigationMenu
66from lp.answers.browser.question import QuestionAddView67from lp.answers.browser.question import QuestionAddView
@@ -355,7 +356,6 @@
355 'homepageurl', 'bugtracker', 'sourceforgeproject',356 'homepageurl', 'bugtracker', 'sourceforgeproject',
356 'freshmeatproject', 'wikiurl']357 'freshmeatproject', 'wikiurl']
357358
358
359 @action('Change Details', name='change')359 @action('Change Details', name='change')
360 def edit(self, action, data):360 def edit(self, action, data):
361 self.updateContextFromData(data)361 self.updateContextFromData(data)
@@ -373,7 +373,7 @@
373class ProjectReviewView(ProjectEditView):373class ProjectReviewView(ProjectEditView):
374374
375 label = "Review upstream project group details"375 label = "Review upstream project group details"
376 field_names = ['name', 'owner', 'active', 'reviewed']376 default_field_names = ['name', 'owner', 'active', 'reviewed']
377377
378 def setUpFields(self):378 def setUpFields(self):
379 """Setup the normal fields from the schema plus adds 'Registrant'.379 """Setup the normal fields from the schema plus adds 'Registrant'.
@@ -382,9 +382,16 @@
382 proper widget created by default. Even though it is read-only, admins382 proper widget created by default. Even though it is read-only, admins
383 need the ability to change it.383 need the ability to change it.
384 """384 """
385 self.field_names = self.default_field_names[:]
386 admin = check_permission('launchpad.Admin', self.context)
387 if not admin:
388 self.field_names.remove('name')
389 self.field_names.remove('owner')
385 super(ProjectReviewView, self).setUpFields()390 super(ProjectReviewView, self).setUpFields()
386 self.form_fields = (self._createAliasesField() + self.form_fields391 self.form_fields = self._createAliasesField() + self.form_fields
387 + self._createRegistrantField())392 if admin:
393 self.form_fields = (
394 self.form_fields + self._createRegistrantField())
388395
389 def _createAliasesField(self):396 def _createAliasesField(self):
390 """Return a PillarAliases field for IProjectGroup.aliases."""397 """Return a PillarAliases field for IProjectGroup.aliases."""
391398
=== modified file 'lib/lp/registry/configure.zcml'
--- lib/lp/registry/configure.zcml 2010-08-19 03:26:03 +0000
+++ lib/lp/registry/configure.zcml 2010-08-22 17:51:44 +0000
@@ -313,9 +313,16 @@
313 attributes="313 attributes="
314 aliases"/>314 aliases"/>
315 <require315 <require
316 permission="launchpad.Admin"316 permission="launchpad.Moderate"
317 attributes="317 attributes="
318 setAliases"/>318 setAliases"/>
319
320 <!-- IProjectGroupModerate -->
321 <allow
322 interface="lp.registry.interfaces.projectgroup.IProjectGroupModerate"/>
323 <require
324 permission="launchpad.Moderate"
325 set_schema="lp.registry.interfaces.projectgroup.IProjectGroupModerate"/>
319 </class>326 </class>
320 <adapter327 <adapter
321 for="lp.registry.interfaces.projectgroup.IProjectGroup"328 for="lp.registry.interfaces.projectgroup.IProjectGroup"
@@ -825,8 +832,12 @@
825 interface="lp.registry.interfaces.person.IPersonEditRestricted"/>832 interface="lp.registry.interfaces.person.IPersonEditRestricted"/>
826 <require833 <require
827 permission="launchpad.Edit"834 permission="launchpad.Edit"
828 set_schema="lp.registry.interfaces.person.IPersonPublic lp.registry.interfaces.person.ITeamPublic"835 set_schema="lp.registry.interfaces.person.IPersonPublic
829 set_attributes="name displayname"/>836 lp.registry.interfaces.person.ITeamPublic"
837 set_attributes="displayname"/>
838 <require
839 permission="launchpad.Moderate"
840 set_attributes="name"/>
830 <require841 <require
831 permission="launchpad.Moderate"842 permission="launchpad.Moderate"
832 interface="lp.registry.interfaces.person.IPersonModerate" />843 interface="lp.registry.interfaces.person.IPersonModerate" />
@@ -837,7 +848,7 @@
837 permission="launchpad.Special"848 permission="launchpad.Special"
838 interface="lp.registry.interfaces.person.IPersonSpecialRestricted"/>849 interface="lp.registry.interfaces.person.IPersonSpecialRestricted"/>
839 <require850 <require
840 permission="launchpad.Edit"851 permission="launchpad.Moderate"
841 set_schema="lp.registry.interfaces.person.IHasStanding"/>852 set_schema="lp.registry.interfaces.person.IHasStanding"/>
842 </class>853 </class>
843854
@@ -1117,8 +1128,9 @@
1117 <!-- https://lists.ubuntu.com/mailman/private/launchpad/2007-April/015189.html1128 <!-- https://lists.ubuntu.com/mailman/private/launchpad/2007-April/015189.html
1118 for further discussion - stub 20070411 -->1129 for further discussion - stub 20070411 -->
11191130
1131 <!-- Per bug 588773, changing to launchpad.Moderate to allow Registry Experts (~registry) -->
1120 <require1132 <require
1121 permission="launchpad.Admin"1133 permission="launchpad.Moderate"
1122 set_attributes="name autoupdate registrant"/>1134 set_attributes="name autoupdate registrant"/>
1123 <require1135 <require
1124 permission="launchpad.TranslationsAdmin"1136 permission="launchpad.TranslationsAdmin"
@@ -1153,7 +1165,7 @@
1153 attributes="1165 attributes="
1154 aliases"/>1166 aliases"/>
1155 <require1167 <require
1156 permission="launchpad.Admin"1168 permission="launchpad.Moderate"
1157 attributes="1169 attributes="
1158 setAliases"/>1170 setAliases"/>
11591171
11601172
=== modified file 'lib/lp/registry/interfaces/projectgroup.py'
--- lib/lp/registry/interfaces/projectgroup.py 2010-08-20 20:31:18 +0000
+++ lib/lp/registry/interfaces/projectgroup.py 2010-08-22 17:51:44 +0000
@@ -97,12 +97,21 @@
97 return IProjectGroup97 return IProjectGroup
9898
9999
100class IProjectGroupModerate(IPillar):
101 """IProjectGroup attributes used with launchpad.Moderate permission."""
102 reviewed = exported(
103 Bool(
104 title=_('Reviewed'), required=False,
105 description=_("Whether or not this project group has been "
106 "reviewed.")))
107
108
100class IProjectGroupPublic(109class IProjectGroupPublic(
101 ICanGetMilestonesDirectly, IHasAppointedDriver, IHasBranches, IHasBugs,110 ICanGetMilestonesDirectly, IHasAppointedDriver, IHasBranches, IHasBugs,
102 IHasDrivers, IHasBranchVisibilityPolicy, IHasIcon, IHasLogo,111 IHasDrivers, IHasBranchVisibilityPolicy, IHasIcon, IHasLogo,
103 IHasMentoringOffers, IHasMergeProposals, IHasMilestones, IHasMugshot,112 IHasMentoringOffers, IHasMergeProposals, IHasMilestones, IHasMugshot,
104 IHasOwner, IHasSpecifications, IHasSprints, IMakesAnnouncements,113 IHasOwner, IHasSpecifications, IHasSprints, IMakesAnnouncements,
105 IKarmaContext, IPillar, IRootContext, IHasOfficialBugTags):114 IKarmaContext, IRootContext, IHasOfficialBugTags):
106 """Public IProjectGroup properties."""115 """Public IProjectGroup properties."""
107116
108 id = Int(title=_('ID'), readonly=True)117 id = Int(title=_('ID'), readonly=True)
@@ -267,12 +276,6 @@
267 "displayed on this project group's home page in Launchpad. "276 "displayed on this project group's home page in Launchpad. "
268 "It should be no bigger than 100kb in size. ")))277 "It should be no bigger than 100kb in size. ")))
269278
270 reviewed = exported(
271 Bool(
272 title=_('Reviewed'), required=False,
273 description=_("Whether or not this project group has been "
274 "reviewed.")))
275
276 bugtracker = exported(279 bugtracker = exported(
277 ReferenceChoice(title=_('Bug Tracker'), required=False,280 ReferenceChoice(title=_('Bug Tracker'), required=False,
278 vocabulary='BugTracker', schema=IBugTracker,281 vocabulary='BugTracker', schema=IBugTracker,
@@ -326,8 +329,10 @@
326 """Return a ProjectGroupSeries object with name `series_name`."""329 """Return a ProjectGroupSeries object with name `series_name`."""
327330
328331
329class IProjectGroup(IProjectGroupPublic, IStructuralSubscriptionTarget,332class IProjectGroup(IProjectGroupPublic,
330 ITranslationPolicy):333 IProjectGroupModerate,
334 IStructuralSubscriptionTarget,
335 ITranslationPolicy):
331 """A ProjectGroup."""336 """A ProjectGroup."""
332337
333 export_as_webservice_entry('project_group')338 export_as_webservice_entry('project_group')
334339
=== modified file 'lib/lp/registry/stories/person/xx-admin-person-review.txt'
--- lib/lp/registry/stories/person/xx-admin-person-review.txt 2010-08-16 19:10:11 +0000
+++ lib/lp/registry/stories/person/xx-admin-person-review.txt 2010-08-22 17:51:44 +0000
@@ -94,8 +94,7 @@
94Registry experts94Registry experts
95----------------95----------------
9696
97Registry experts can see the +review page in full, just like Launchpad97Registry experts can see the +review page, minus the displayname.
98admins.
9998
100 >>> email = "expert@example.com"99 >>> email = "expert@example.com"
101 >>> password = "test"100 >>> password = "test"
@@ -106,10 +105,24 @@
106 >>> print expert_browser.title105 >>> print expert_browser.title
107 Review person...106 Review person...
108107
108 >>> expert_browser.getControl('Name', index=0).value = "no-way"
109 >>> expert_browser.getControl(
110 ... 'Display Name', index=0)
111 Traceback (most recent call last):
112 ...
113 LookupError: label 'Display Name'
114
115 >>> expert_browser.getControl('Personal standing').value = ['GOOD']
116 >>> expert_browser.getControl(
117 ... name='field.personal_standing_reason').value = 'good guy'
118 >>> expert_browser.getControl('Change').click()
119 >>> print expert_browser.url
120 http://launchpad.dev/~no-way
121
109However, members of the registry team only get partial access to the122However, members of the registry team only get partial access to the
110review account page to be able to suspend spam accounts.123review account page to be able to suspend spam accounts.
111124
112 >>> expert_browser.open('http://launchpad.dev/~no-priv/+reviewaccount')125 >>> expert_browser.open('http://launchpad.dev/~no-way/+reviewaccount')
113 >>> print expert_browser.title126 >>> print expert_browser.title
114 Review person's account...127 Review person's account...
115128
116129
=== modified file 'lib/lp/registry/stories/product/xx-product-edit.txt'
--- lib/lp/registry/stories/product/xx-product-edit.txt 2010-08-13 21:30:24 +0000
+++ lib/lp/registry/stories/product/xx-product-edit.txt 2010-08-22 17:51:44 +0000
@@ -235,22 +235,50 @@
235 ...235 ...
236 Unauthorized:...236 Unauthorized:...
237237
238Tf we add them to the Registry Experts team:238If we add them to the Registry Experts team:
239239
240 >>> admin_browser.open("http://launchpad.dev/~registry/+addmember")240 >>> admin_browser.open("http://launchpad.dev/~registry/+addmember")
241 >>> admin_browser.getControl('New member').value = 'no-priv'241 >>> admin_browser.getControl('New member').value = 'no-priv'
242 >>> admin_browser.getControl('Add Member').click()242 >>> admin_browser.getControl('Add Member').click()
243243
244They cannot edit projects.244They still cannot edit projects.
245245
246 >>> browser.open('http://launchpad.dev/firefox/+edit')246 >>> browser.open('http://launchpad.dev/firefox/+edit')
247 Traceback (most recent call last):247 Traceback (most recent call last):
248 ...248 ...
249 Unauthorized:...249 Unauthorized:...
250250
251But they still can access +admin.251But they can access +admin, though it is more restricted than that for admins.
252252
253 >>> browser.open('http://launchpad.dev/firefox/+admin')253 >>> from lp.testing import login, logout
254 >>> login('admin@canonical.com')
255 >>> product = factory.makeProduct(name='trebuche')
256 >>> logout()
257
258The registry experts do not have access to the maintainer or
259registrant fields.
260
261 >>> browser.open('http://launchpad.dev/trebuche/+admin')
262 >>> browser.getControl('Maintainer')
263 Traceback (most recent call last):
264 ...
265 LookupError: label 'Maintainer'
266 >>> browser.getControl('Registrant')
267 Traceback (most recent call last):
268 ...
269 LookupError: label 'Registrant'
270
271But registry experts can change a product name and set an alias.
272
273 >>> browser.getControl('Name').value = 'trebuchet'
274 >>> browser.getControl('Aliases').value = 'trebucket'
275 >>> browser.getControl('Change').click()
276
277 >>> browser.getLink('Administer').click()
278 >>> print browser.getControl('Name').value
279 trebuchet
280 >>> print browser.getControl('Aliases').value
281 trebucket
254282
255283
256Display error when trying to remove all licenses284Display error when trying to remove all licenses
257285
=== modified file 'lib/lp/registry/stories/project/xx-project-edit.txt'
--- lib/lp/registry/stories/project/xx-project-edit.txt 2010-08-16 19:10:11 +0000
+++ lib/lp/registry/stories/project/xx-project-edit.txt 2010-08-22 17:51:44 +0000
@@ -132,9 +132,33 @@
132 ...132 ...
133 Unauthorized...133 Unauthorized...
134134
135Registry experts do have full access to administer project groups.135Registry experts do have access to administer project groups, though
136there are fewer fields available.
136137
137 >>> expert_browser.open('http://launchpad.dev/new-name')138 >>> expert_browser.open('http://launchpad.dev/new-name')
138 >>> expert_browser.getLink('Administer').click()139 >>> expert_browser.getLink('Administer').click()
139 >>> print expert_browser.url140 >>> print expert_browser.url
140 http://launchpad.dev/new-name/+review141 http://launchpad.dev/new-name/+review
142
143 >>> expert_browser.getControl('Maintainer')
144 Traceback (most recent call last):
145 ...
146 LookupError: label 'Maintainer'
147 >>> expert_browser.getControl('Registrant')
148 Traceback (most recent call last):
149 ...
150 LookupError: label 'Registrant'
151 >>> expert_browser.getControl('Name')
152 Traceback (most recent call last):
153 ...
154 LookupError: label 'Name'
155
156 >>> expert_browser.getControl('Aliases').value = 'sleepy'
157 >>> expert_browser.getControl('Active').selected = False
158 >>> expert_browser.getControl('Reviewed').selected = False
159 >>> expert_browser.getControl('Change').click()
160
161 >>> expert_browser.open('http://launchpad.dev/new-name')
162 >>> expert_browser.getLink('Administer').click()
163 >>> print expert_browser.getControl('Aliases').value
164 sleepy