launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #02161
[Merge] lp:~bac/launchpad/bug-662994 into lp:launchpad
Brad Crittenden has proposed merging lp:~bac/launchpad/bug-662994 into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
#662994 ~registry cannot rename a project group
https://bugs.launchpad.net/bugs/662994
= Summary =
A task for CHR and Curtis is to change project group names. The
registry team has never had the ability to do so.
== Proposed fix ==
Move the permission of name from launchpad.Edit to launchpad.Moderate
and from the +edit to the +review page.
== Pre-implementation notes ==
Talk with Curtis
== Implementation details ==
The change described gives ~registry the ability to change a PG name but
removes it from the PG owner. This effect is necessary due to the way
our permissions are partitioned between owners, admins, and registry.
On a positive note, it brings behavior of PG editing in-line with projects.
I also changed the matchers to expose 'Contains' in __all__ which had
been inadvertently left out.
== Tests ==
bin/test -vvm lp.registry -t xx-project-edit.txt -t test_projectgroup
== Demo and Q/A ==
= Launchpad lint =
I will fix these lint issues.
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/registry/stories/project/xx-project-edit.txt
lib/lp/registry/tests/test_projectgroup.py
lib/lp/registry/stories/object/xx-nameblacklist.txt
lib/lp/registry/interfaces/projectgroup.py
lib/lp/registry/browser/tests/test_projectgroup.py
lib/lp/testing/matchers.py
lib/lp/registry/browser/project.py
./lib/lp/registry/stories/object/xx-nameblacklist.txt
7: source has bad indentation.
14: source exceeds 78 characters.
20: source has bad indentation.
24: source exceeds 78 characters.
30: source has bad indentation.
39: source has bad indentation.
56: source has bad indentation.
61: source has bad indentation.
./lib/lp/testing/matchers.py
144: E231 missing whitespace after ':'
236: Line exceeds 78 characters.
--
https://code.launchpad.net/~bac/launchpad/bug-662994/+merge/43647
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~bac/launchpad/bug-662994 into lp:launchpad.
=== modified file 'lib/lp/registry/browser/project.py'
--- lib/lp/registry/browser/project.py 2010-12-01 04:27:44 +0000
+++ lib/lp/registry/browser/project.py 2010-12-14 14:41:27 +0000
@@ -350,11 +350,11 @@
class ProjectEditView(LaunchpadEditFormView):
"""View class that lets you edit a Project object."""
implements(IProjectGroupEditMenu)
-
label = "Change project group details"
+ page_title = label
schema = IProjectGroup
field_names = [
- 'name', 'displayname', 'title', 'summary', 'description',
+ 'displayname', 'title', 'summary', 'description',
'bug_reporting_guidelines', 'bug_reported_acknowledgement',
'homepageurl', 'bugtracker', 'sourceforgeproject',
'freshmeatproject', 'wikiurl']
@@ -388,8 +388,10 @@
self.field_names = self.default_field_names[:]
admin = check_permission('launchpad.Admin', self.context)
if not admin:
+ self.field_names.remove('owner')
+ moderator = check_permission('launchpad.Moderate', self.context)
+ if not moderator:
self.field_names.remove('name')
- self.field_names.remove('owner')
super(ProjectReviewView, self).setUpFields()
self.form_fields = self._createAliasesField() + self.form_fields
if admin:
=== added file 'lib/lp/registry/browser/tests/test_projectgroup.py'
--- lib/lp/registry/browser/tests/test_projectgroup.py 1970-01-01 00:00:00 +0000
+++ lib/lp/registry/browser/tests/test_projectgroup.py 2010-12-14 14:41:27 +0000
@@ -0,0 +1,84 @@
+# Copyright 2010 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Tests for project group views."""
+
+__metaclass__ = type
+
+from zope.component import getUtility
+from zope.security.interfaces import Unauthorized
+from testtools.matchers import Not
+from canonical.launchpad.webapp import canonical_url
+from canonical.launchpad.webapp.interfaces import ILaunchBag
+from canonical.testing.layers import DatabaseFunctionalLayer
+from lp.registry.interfaces.person import IPersonSet
+from lp.testing import (
+ celebrity_logged_in,
+ person_logged_in,
+ TestCaseWithFactory,
+ )
+from lp.testing.matchers import (
+ Contains,
+ )
+from lp.testing.sampledata import ADMIN_EMAIL
+from lp.testing.views import create_initialized_view
+
+
+class TestProjectGroupEditView(TestCaseWithFactory):
+ """Tests the edit view."""
+
+ layer = DatabaseFunctionalLayer
+
+ def setUp(self):
+ super(TestProjectGroupEditView, self).setUp()
+ self.project_group = self.factory.makeProject(name='grupo')
+
+ def test_links_admin(self):
+ # An admin can change details and administer a project group.
+ with celebrity_logged_in('admin'):
+ user = getUtility(ILaunchBag).user
+ view = create_initialized_view(self.project_group, '+index',
+ principal=user)
+ contents = view.render()
+ self.assertThat(contents, Contains("Change details"))
+ self.assertThat(contents, Contains("Administer"))
+
+ def test_links_registry_expert(self):
+ # A registry expert cannot change details but can administer a project
+ # group.
+ with celebrity_logged_in('registry_experts'):
+ user = getUtility(ILaunchBag).user
+ view = create_initialized_view(self.project_group, '+index',
+ principal=user)
+ contents = view.render()
+ self.assertThat(contents, Not(Contains("Change details")))
+ self.assertThat(contents, Contains("Administer"))
+
+ def test_links_owner(self):
+ # An owner can change details but not administer a project group.
+ with person_logged_in(self.project_group.owner):
+ user = getUtility(ILaunchBag).user
+ view = create_initialized_view(self.project_group, '+index',
+ principal=user)
+ contents = view.render()
+ self.assertThat(contents, Contains("Change details"))
+ self.assertThat(contents, Not(Contains("Administer")))
+
+ def test_edit_view_admin(self):
+ admin = getUtility(IPersonSet).getByEmail(ADMIN_EMAIL)
+ browser = self.getUserBrowser(user=admin)
+ browser.open(canonical_url(self.project_group, view_name='+edit'))
+ browser.open(canonical_url(self.project_group, view_name='+review'))
+
+ def test_edit_view_registry_expert(self):
+ registry_expert = self.factory.makeRegistryExpert()
+ browser = self.getUserBrowser(user=registry_expert)
+ url = canonical_url(self.project_group, view_name='+edit')
+ self.assertRaises(Unauthorized, browser.open, url)
+ browser.open(canonical_url(self.project_group, view_name='+review'))
+
+ def test_edit_view_owner(self):
+ browser = self.getUserBrowser(user=self.project_group.owner)
+ browser.open(canonical_url(self.project_group, view_name='+edit'))
+ url = canonical_url(self.project_group, view_name='+review')
+ self.assertRaises(Unauthorized, browser.open, url)
=== modified file 'lib/lp/registry/interfaces/projectgroup.py'
--- lib/lp/registry/interfaces/projectgroup.py 2010-12-01 20:53:19 +0000
+++ lib/lp/registry/interfaces/projectgroup.py 2010-12-14 14:41:27 +0000
@@ -104,6 +104,15 @@
title=_('Reviewed'), required=False,
description=_("Whether or not this project group has been "
"reviewed.")))
+ name = exported(
+ ProjectNameField(
+ title=_('Name'),
+ required=True,
+ description=_(
+ "A unique name, used in URLs, identifying the project "
+ "group. All lowercase, no special characters. "
+ "Examples: apache, mozilla, gimp."),
+ constraint=name_validator))
class IProjectGroupPublic(
@@ -133,16 +142,6 @@
description=_("Project group registrant. Must be a valid "
"Launchpad Person.")))
- name = exported(
- ProjectNameField(
- title=_('Name'),
- required=True,
- description=_(
- "A unique name, used in URLs, identifying the project "
- "group. All lowercase, no special characters. "
- "Examples: apache, mozilla, gimp."),
- constraint=name_validator))
-
displayname = exported(
TextLine(
title=_('Display Name'),
=== modified file 'lib/lp/registry/stories/object/xx-nameblacklist.txt'
--- lib/lp/registry/stories/object/xx-nameblacklist.txt 2010-11-03 20:38:32 +0000
+++ lib/lp/registry/stories/object/xx-nameblacklist.txt 2010-12-14 14:41:27 +0000
@@ -18,7 +18,7 @@
Try renaming a project to a blacklisted name:
>>> admin_browser.open('http://launchpad.dev/mozilla')
->>> admin_browser.getLink('Change details').click()
+>>> admin_browser.getLink('Administer').click()
>>> admin_browser.getControl('Name', index=0).value = 'blacklisted'
>>> admin_browser.getControl('Change Details').click()
>>> "The name 'blacklisted' has been blocked by the Launchpad administrators" \
@@ -51,7 +51,7 @@
be a Launchpad Celebrity.
We can edit the details of an object with a blacklisted name quite
-happily without generating
+happily without generating
>>> admin_browser.open('http://launchpad.dev/~admins')
>>> admin_browser.getLink('Change details').click()
@@ -62,4 +62,3 @@
[]
>>> "has been blocked" in admin_browser.contents
False
-
=== modified file 'lib/lp/registry/stories/project/xx-project-edit.txt'
--- lib/lp/registry/stories/project/xx-project-edit.txt 2010-08-20 18:13:20 +0000
+++ lib/lp/registry/stories/project/xx-project-edit.txt 2010-12-14 14:41:27 +0000
@@ -10,7 +10,6 @@
Change project group details...
>>> soup = find_main_content(browser.contents)
- >>> browser.getControl(name='field.name').value = 'new-name'
>>> browser.getControl('Display Name').value = 'New Name'
>>> browser.getControl('Title').value = 'New Title'
>>> browser.getControl('Project Group Summary').value = 'New Summary.'
@@ -20,7 +19,7 @@
>>> browser.getControl('Change Details').click()
>>> print browser.url
- http://launchpad.dev/new-name
+ http://launchpad.dev/gnome
Regular users can't access the +review page.
@@ -31,18 +30,19 @@
But administrators can access the page:
- >>> admin_browser.open('http://launchpad.dev/new-name')
+ >>> admin_browser.open('http://launchpad.dev/gnome')
>>> admin_browser.getLink('Administer').click()
>>> print admin_browser.url
- http://launchpad.dev/new-name/+review
+ http://launchpad.dev/gnome/+review
>>> print admin_browser.title
Review upstream project group...
-Mark the project as reviewed.
+Mark the project as reviewed and change the name.
>>> admin_browser.getControl('Reviewed').selected = True
+ >>> admin_browser.getControl(name='field.name').value = 'new-name'
>>> admin_browser.getControl('Change').click()
>>> print admin_browser.url
@@ -148,17 +148,14 @@
Traceback (most recent call last):
...
LookupError: label 'Registrant'
- >>> expert_browser.getControl('Name')
- Traceback (most recent call last):
- ...
- LookupError: label 'Name'
+ >>> expert_browser.getControl('Name').value = 'newer-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.open('http://launchpad.dev/newer-name')
>>> expert_browser.getLink('Administer').click()
>>> print expert_browser.getControl('Aliases').value
sleepy
=== renamed file 'lib/lp/registry/tests/test_project.py' => 'lib/lp/registry/tests/test_projectgroup.py'
--- lib/lp/registry/tests/test_project.py 2010-12-08 17:22:23 +0000
+++ lib/lp/registry/tests/test_projectgroup.py 2010-12-14 14:41:27 +0000
@@ -7,8 +7,8 @@
from lazr.restfulclient.errors import ClientError
from zope.component import getUtility
+from zope.security.interfaces import Unauthorized
-from canonical.launchpad.ftests import login
from canonical.launchpad.webapp.errorlog import globalErrorUtility
from canonical.testing.layers import (
DatabaseFunctionalLayer,
@@ -17,6 +17,7 @@
from lp.registry.interfaces.projectgroup import IProjectGroupSet
from lp.testing import (
launchpadlib_for,
+ login_celebrity,
login_person,
TestCaseWithFactory,
)
@@ -108,6 +109,33 @@
self.assertEqual(self.project3, results[0])
+class TestProjectGroupPermissions(TestCaseWithFactory):
+
+ layer = DatabaseFunctionalLayer
+
+ def setUp(self):
+ super(TestProjectGroupPermissions, self).setUp()
+ self.pg = self.factory.makeProject(name='my-project-group')
+
+ def test_attribute_changes_by_admin(self):
+ login_celebrity('admin')
+ self.pg.name = 'new-name'
+ self.pg.owner = self.factory.makePerson(name='project-group-owner')
+
+ def test_attribute_changes_by_registry_admin(self):
+ login_celebrity('registry_experts')
+ new_owner = self.factory.makePerson(name='project-group-owner')
+ self.pg.name = 'new-name'
+ self.assertRaises(Unauthorized,
+ setattr, self.pg, 'owner', new_owner)
+
+ def test_attribute_changes_by_owner(self):
+ login_person(self.pg.owner)
+ self.assertRaises(Unauthorized,
+ setattr, self.pg, 'name', 'new-name')
+ self.pg.owner = self.factory.makePerson(name='project-group-owner')
+
+
class TestLaunchpadlibAPI(TestCaseWithFactory):
layer = DatabaseFunctionalLayer
@@ -117,8 +145,8 @@
# is raised when trying to deactivate a project that has source
# releases.
last_oops = globalErrorUtility.getLastOopsReport()
+
launchpad = launchpadlib_for("test", "salgado", "WRITE_PUBLIC")
-
project = launchpad.projects['evolution']
project.active = False
e = self.assertRaises(ClientError, project.lp_save)
=== modified file 'lib/lp/testing/matchers.py'
--- lib/lp/testing/matchers.py 2010-10-19 22:28:51 +0000
+++ lib/lp/testing/matchers.py 2010-12-14 14:41:27 +0000
@@ -3,6 +3,7 @@
__metaclass__ = type
__all__ = [
+ 'Contains',
'DoesNotCorrectlyProvide',
'DoesNotProvide',
'HasQueryCount',
@@ -141,7 +142,7 @@
for query in self.query_collector.queries:
result.append(unicode(query).encode('utf8'))
return {'queries': Content(UTF8_TEXT, lambda:['\n'.join(result)])}
-
+
class IsNotProxied(Mismatch):
"""An object is not proxied."""
