launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #00689
[Merge] lp:~bac/launchpad/bug-588773-charm into lp:launchpad/devel
Brad Crittenden has proposed merging lp:~bac/launchpad/bug-588773-charm into lp:launchpad/devel.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
#588773 Allow Registry Admins to change pillar and person names
https://bugs.launchpad.net/bugs/588773
= 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
--
https://code.launchpad.net/~bac/launchpad/bug-588773-charm/+merge/33248
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~bac/launchpad/bug-588773-charm into lp:launchpad/devel.
=== modified file 'lib/lp/registry/browser/product.py'
--- lib/lp/registry/browser/product.py 2010-08-19 03:26:03 +0000
+++ lib/lp/registry/browser/product.py 2010-08-20 18:31:03 +0000
@@ -1474,7 +1474,13 @@
class ProductAdminView(ProductEditView, ProductValidationMixin):
"""View for $project/+admin"""
label = "Administer project details"
- field_names = ["name", "owner", "active", "autoupdate", "private_bugs"]
+ default_field_names = [
+ "name",
+ "owner",
+ "active",
+ "autoupdate",
+ "private_bugs",
+ ]
@property
def page_title(self):
@@ -1488,9 +1494,16 @@
proper widget created by default. Even though it is read-only, admins
need the ability to change it.
"""
+ self.field_names = self.default_field_names[:]
+ admin = check_permission('launchpad.Admin', self.context)
+ if not admin:
+ self.field_names.remove('owner')
+ self.field_names.remove('autoupdate')
super(ProductAdminView, self).setUpFields()
- self.form_fields = (self._createAliasesField() + self.form_fields
- + self._createRegistrantField())
+ self.form_fields = self._createAliasesField() + self.form_fields
+ if admin:
+ self.form_fields = (
+ self.form_fields + self._createRegistrantField())
def _createAliasesField(self):
"""Return a PillarAliases field for IProduct.aliases."""
=== modified file 'lib/lp/registry/browser/project.py'
--- lib/lp/registry/browser/project.py 2010-08-19 03:26:03 +0000
+++ lib/lp/registry/browser/project.py 2010-08-20 18:31:03 +0000
@@ -43,6 +43,7 @@
from canonical.cachedproperty import cachedproperty
from canonical.launchpad import _
from lp.app.errors import NotFoundError
+from canonical.launchpad.webapp.authorization import check_permission
from canonical.launchpad.webapp.menu import NavigationMenu
from lp.blueprints.browser.specificationtarget import (
HasSpecificationsMenuMixin)
@@ -328,7 +329,6 @@
'homepageurl', 'bugtracker', 'sourceforgeproject',
'freshmeatproject', 'wikiurl']
-
@action('Change Details', name='change')
def edit(self, action, data):
self.updateContextFromData(data)
@@ -346,7 +346,7 @@
class ProjectReviewView(ProjectEditView):
label = "Review upstream project group details"
- field_names = ['name', 'owner', 'active', 'reviewed']
+ default_field_names = ['name', 'owner', 'active', 'reviewed']
def setUpFields(self):
"""Setup the normal fields from the schema plus adds 'Registrant'.
@@ -355,9 +355,16 @@
proper widget created by default. Even though it is read-only, admins
need the ability to change it.
"""
+ self.field_names = self.default_field_names[:]
+ admin = check_permission('launchpad.Admin', self.context)
+ if not admin:
+ self.field_names.remove('name')
+ self.field_names.remove('owner')
super(ProjectReviewView, self).setUpFields()
- self.form_fields = (self._createAliasesField() + self.form_fields
- + self._createRegistrantField())
+ self.form_fields = self._createAliasesField() + self.form_fields
+ if admin:
+ self.form_fields = (
+ self.form_fields + self._createRegistrantField())
def _createAliasesField(self):
"""Return a PillarAliases field for IProjectGroup.aliases."""
=== 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-20 18:31:03 +0000
@@ -313,9 +313,16 @@
attributes="
aliases"/>
<require
- permission="launchpad.Admin"
+ permission="launchpad.Moderate"
attributes="
setAliases"/>
+
+ <!-- IProjectGroupModerate -->
+ <allow
+ interface="lp.registry.interfaces.projectgroup.IProjectGroupModerate"/>
+ <require
+ permission="launchpad.Moderate"
+ set_schema="lp.registry.interfaces.projectgroup.IProjectGroupModerate"/>
</class>
<adapter
for="lp.registry.interfaces.projectgroup.IProjectGroup"
@@ -825,7 +832,10 @@
interface="lp.registry.interfaces.person.IPersonEditRestricted"/>
<require
permission="launchpad.Edit"
- set_schema="lp.registry.interfaces.person.IPersonPublic lp.registry.interfaces.person.ITeamPublic"
+ set_schema="lp.registry.interfaces.person.IPersonPublic
+ lp.registry.interfaces.person.ITeamPublic"/>
+ <require
+ permission="launchpad.Moderate"
set_attributes="name displayname"/>
<require
permission="launchpad.Moderate"
@@ -1117,8 +1127,9 @@
<!-- https://lists.ubuntu.com/mailman/private/launchpad/2007-April/015189.html
for further discussion - stub 20070411 -->
+ <!-- Per bug 588773, changing to launchpad.Moderate to allow Registry Experts (~registry) -->
<require
- permission="launchpad.Admin"
+ permission="launchpad.Moderate"
set_attributes="name autoupdate registrant"/>
<require
permission="launchpad.TranslationsAdmin"
@@ -1153,7 +1164,7 @@
attributes="
aliases"/>
<require
- permission="launchpad.Admin"
+ permission="launchpad.Moderate"
attributes="
setAliases"/>
=== modified file 'lib/lp/registry/interfaces/projectgroup.py'
--- lib/lp/registry/interfaces/projectgroup.py 2010-08-11 15:35:29 +0000
+++ lib/lp/registry/interfaces/projectgroup.py 2010-08-20 18:31:03 +0000
@@ -60,12 +60,21 @@
return IProjectGroup
+class IProjectGroupModerate(IPillar):
+ """IProjectGroup attributes used with launchpad.Moderate permission."""
+ reviewed = exported(
+ Bool(
+ title=_('Reviewed'), required=False,
+ description=_("Whether or not this project group has been "
+ "reviewed.")))
+
+
class IProjectGroupPublic(
ICanGetMilestonesDirectly, IHasAppointedDriver, IHasBranches, IHasBugs,
IHasDrivers, IHasBranchVisibilityPolicy, IHasIcon, IHasLogo,
IHasMentoringOffers, IHasMergeProposals, IHasMilestones, IHasMugshot,
IHasOwner, IHasSpecifications, IHasSprints, IMakesAnnouncements,
- IKarmaContext, IPillar, IRootContext, IHasOfficialBugTags):
+ IKarmaContext, IRootContext, IHasOfficialBugTags):
"""Public IProjectGroup properties."""
id = Int(title=_('ID'), readonly=True)
@@ -230,12 +239,6 @@
"displayed on this project group's home page in Launchpad. "
"It should be no bigger than 100kb in size. ")))
- reviewed = exported(
- Bool(
- title=_('Reviewed'), required=False,
- description=_("Whether or not this project group has been "
- "reviewed.")))
-
bugtracker = exported(
ReferenceChoice(title=_('Bug Tracker'), required=False,
vocabulary='BugTracker', schema=IBugTracker,
@@ -289,8 +292,10 @@
"""Return a ProjectGroupSeries object with name `series_name`."""
-class IProjectGroup(IProjectGroupPublic, IStructuralSubscriptionTarget,
- ITranslationPolicy):
+class IProjectGroup(IProjectGroupPublic,
+ IProjectGroupModerate,
+ IStructuralSubscriptionTarget,
+ ITranslationPolicy):
"""A ProjectGroup."""
export_as_webservice_entry('project_group')
=== 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-20 18:31:03 +0000
@@ -106,10 +106,19 @@
>>> print expert_browser.title
Review person...
+ >>> expert_browser.getControl('Name', index=0).value = "no-way"
+ >>> expert_browser.getControl(
+ ... 'Display Name', index=0).value = 'Ray'
+ >>> expert_browser.getControl('Change').click()
+ >>> print expert_browser.title
+ Ray in Launchpad
+ >>> print expert_browser.url
+ http://launchpad.dev/~no-way
+
However, members of the registry team only get partial access to the
review account page to be able to suspend spam accounts.
- >>> expert_browser.open('http://launchpad.dev/~no-priv/+reviewaccount')
+ >>> expert_browser.open('http://launchpad.dev/~no-way/+reviewaccount')
>>> print expert_browser.title
Review person's account...
=== 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-20 18:31:03 +0000
@@ -235,22 +235,50 @@
...
Unauthorized:...
-Tf we add them to the Registry Experts team:
+If we add them to the Registry Experts team:
>>> admin_browser.open("http://launchpad.dev/~registry/+addmember";)
>>> admin_browser.getControl('New member').value = 'no-priv'
>>> admin_browser.getControl('Add Member').click()
-They cannot edit projects.
+They still cannot edit projects.
>>> browser.open('http://launchpad.dev/firefox/+edit')
Traceback (most recent call last):
...
Unauthorized:...
-But they still can access +admin.
-
- >>> browser.open('http://launchpad.dev/firefox/+admin')
+But they can access +admin, though it is more restricted than that for admins.
+
+ >>> from lp.testing import login, logout
+ >>> login('admin@xxxxxxxxxxxxx')
+ >>> product = factory.makeProduct(name='trebuche')
+ >>> logout()
+
+The registry experts do not have access to the maintainer or
+registrant fields.
+
+ >>> browser.open('http://launchpad.dev/trebuche/+admin')
+ >>> browser.getControl('Maintainer')
+ Traceback (most recent call last):
+ ...
+ LookupError: label 'Maintainer'
+ >>> browser.getControl('Registrant')
+ Traceback (most recent call last):
+ ...
+ LookupError: label 'Registrant'
+
+But registry experts can change a product name and set an alias.
+
+ >>> browser.getControl('Name').value = 'trebuchet'
+ >>> browser.getControl('Aliases').value = 'trebucket'
+ >>> browser.getControl('Change').click()
+
+ >>> browser.getLink('Administer').click()
+ >>> print browser.getControl('Name').value
+ trebuchet
+ >>> print browser.getControl('Aliases').value
+ trebucket
Display error when trying to remove all licenses
=== 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-20 18:31:03 +0000
@@ -132,9 +132,33 @@
...
Unauthorized...
-Registry experts do have full access to administer project groups.
+Registry experts do have access to administer project groups, though
+there are fewer fields available.
>>> expert_browser.open('http://launchpad.dev/new-name')
>>> expert_browser.getLink('Administer').click()
>>> print expert_browser.url
http://launchpad.dev/new-name/+review
+
+ >>> expert_browser.getControl('Maintainer')
+ Traceback (most recent call last):
+ ...
+ LookupError: label 'Maintainer'
+ >>> expert_browser.getControl('Registrant')
+ Traceback (most recent call last):
+ ...
+ LookupError: label 'Registrant'
+ >>> expert_browser.getControl('Name')
+ Traceback (most recent call last):
+ ...
+ LookupError: label 'Name'
+
+ >>> expert_browser.getControl('Aliases').value = 'sleepy'
+ >>> expert_browser.getControl('Active').selected = False
+ >>> expert_browser.getControl('Reviewed').selected = False
+ >>> expert_browser.getControl('Change').click()
+
+ >>> expert_browser.open('http://launchpad.dev/new-name')
+ >>> expert_browser.getLink('Administer').click()
+ >>> print expert_browser.getControl('Aliases').value
+ sleepy
