launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #06203
[Merge] lp:~sinzui/launchpad/mailing-list-name-field into lp:launchpad
Curtis Hovey has proposed merging lp:~sinzui/launchpad/mailing-list-name-field into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~sinzui/launchpad/mailing-list-name-field/+merge/90479
iDo not imply a team can be renamed when it has a mailing list.
Launchpad bug: https://bugs.launchpad.net/bugs/889496
Pre-implementation: myself
The field to rename was shown implying I could do the rename. The form
did not check that there was an mailing list. I was confused actually,
because the team page did not show an list. This list was being created
:(.
This is not an issue with the +edit page. I was using the +review page.
I think the two view need to share some code.
* +review implies I can change the name of a user with an active PPA...
PersonRenameFormMixin. This is broken for users and teams.
* There are two options. A. subclass TeamEditView or
create a new class using TeamFormMixin, PersonRenameFormMixin.
--------------------------------------------------------------------
RULES
* Extract the rename tests from TestPersonEditView to a mixin.
* Add a testcase for PersonAdministerView that used the new mixin.
* Update PersonAdministerView to extend PersonRenameFormMixin
* Extract the rename tests from TestTeamEditView to a mixin.
* Add a test for for TeamAdministerView that uses the mixin.
* Create TeamAdministerView from PersonAdministerViewthat does
not have standing.
QA
* Visit https://qastaging.launchpad.net/~test234/+review
* Verify that the name field is not editable.
* Visit https://qastaging.launchpad.net/~sinzui/+review
* Verify that the name field is not editable.
LINT
lib/lp/registry/browser/configure.zcml
lib/lp/registry/browser/person.py
lib/lp/registry/browser/team.py
lib/lp/registry/browser/tests/test_person_view.py
lib/lp/registry/browser/tests/test_team.py
TEST
./bin/test -vv -t TestPersonEditView -t PersonAdministerViewTestCase \
lp.registry.browser.tests.test_person_view
./bin/test -vvc -t TestTeamEditView -t TeamAdminisiterViewTestCase \
lp.registry.browser.tests.test_team
IMPLEMENTATION
Updated PersonAdministerView to use PersonRenameFormMixin. Extracted the
common rename tests to a mixin and used it in both the edit and
administer test cases. I had to move PersonRenameFormMixin which inflated
the diff. I modernised the setup of the mixing tests. I added tests about
which fields are visible to admins and registry_experts because I did not
see the rules verified in other browser tests.
lib/lp/registry/browser/person.py
lib/lp/registry/browser/tests/test_person_view.py
Subclassed PersonAdministerView so that I do not see the user standing
fields when looking at a team. Extracted the common rename tests to a mixin
and used it in both the edit and administer test cases.
lib/lp/registry/browser/configure.zcml
lib/lp/registry/browser/team.py
lib/lp/registry/browser/tests/test_team.py
--
https://code.launchpad.net/~sinzui/launchpad/mailing-list-name-field/+merge/90479
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~sinzui/launchpad/mailing-list-name-field into lp:launchpad.
=== modified file 'lib/lp/registry/browser/configure.zcml'
--- lib/lp/registry/browser/configure.zcml 2012-01-18 16:49:09 +0000
+++ lib/lp/registry/browser/configure.zcml 2012-01-27 16:44:35 +0000
@@ -1152,6 +1152,12 @@
permission="launchpad.Edit"
template="../templates/team-edit.pt"/>
<browser:page
+ name="+review"
+ for="lp.registry.interfaces.person.ITeam"
+ class="lp.registry.browser.team.TeamAdministerView"
+ permission="launchpad.Moderate"
+ template="../templates/person-review.pt"/>
+ <browser:page
name="+branding"
for="lp.registry.interfaces.person.ITeam"
class="lp.registry.browser.team.TeamBrandingView"
=== modified file 'lib/lp/registry/browser/person.py'
--- lib/lp/registry/browser/person.py 2012-01-25 04:56:50 +0000
+++ lib/lp/registry/browser/person.py 2012-01-27 16:44:35 +0000
@@ -1225,7 +1225,24 @@
return encodeddata
-class PersonAdministerView(LaunchpadEditFormView):
+class PersonRenameFormMixin(LaunchpadEditFormView):
+
+ def setUpWidgets(self):
+ """See `LaunchpadViewForm`.
+
+ Renames are prohibited if a person/team has an active PPA or an
+ active mailing list.
+ """
+ reason = self.context.checkRename()
+ if reason:
+ # This makes the field's widget display (i.e. read) only.
+ self.form_fields['name'].for_display = True
+ super(PersonRenameFormMixin, self).setUpWidgets()
+ if reason:
+ self.widgets['name'].hint = reason
+
+
+class PersonAdministerView(PersonRenameFormMixin):
"""Administer an `IPerson`."""
schema = IPerson
label = "Review person"
@@ -3475,23 +3492,6 @@
page_title = label
-class PersonRenameFormMixin(LaunchpadEditFormView):
-
- def setUpWidgets(self):
- """See `LaunchpadViewForm`.
-
- Renames are prohibited if a person/team has an active PPA or an
- active mailing list.
- """
- reason = self.context.checkRename()
- if reason:
- # This makes the field's widget display (i.e. read) only.
- self.form_fields['name'].for_display = True
- super(PersonRenameFormMixin, self).setUpWidgets()
- if reason:
- self.widgets['name'].hint = reason
-
-
class PersonEditView(PersonRenameFormMixin, BasePersonEditView):
"""The Person 'Edit' page."""
=== modified file 'lib/lp/registry/browser/team.py'
--- lib/lp/registry/browser/team.py 2012-01-25 04:56:50 +0000
+++ lib/lp/registry/browser/team.py 2012-01-27 16:44:35 +0000
@@ -95,6 +95,7 @@
from lp.registry.browser.objectreassignment import ObjectReassignmentView
from lp.registry.browser.person import (
CommonMenuLinks,
+ PersonAdministerView,
PersonIndexView,
PersonNavigation,
PersonRenameFormMixin,
@@ -353,6 +354,12 @@
cancel_url = next_url
+class TeamAdministerView(PersonAdministerView):
+ """A view to administer teams on behalf of users."""
+ label = "Review team"
+ default_field_names = ['name', 'displayname']
+
+
def generateTokenAndValidationEmail(email, team):
"""Send a validation message to the given email."""
login = getUtility(ILaunchBag).login
=== modified file 'lib/lp/registry/browser/tests/test_person_view.py'
--- lib/lp/registry/browser/tests/test_person_view.py 2012-01-20 15:42:44 +0000
+++ lib/lp/registry/browser/tests/test_person_view.py 2012-01-27 16:44:35 +0000
@@ -21,10 +21,7 @@
from lp.app.interfaces.launchpad import ILaunchpadCelebrities
from lp.bugs.model.bugtask import BugTask
from lp.buildmaster.enums import BuildStatus
-from lp.registry.browser.person import (
- PersonEditView,
- PersonView,
- )
+from lp.registry.browser.person import PersonView
from lp.registry.browser.team import TeamInvitationView
from lp.registry.interfaces.karma import IKarmaCacheManager
from lp.registry.interfaces.person import (
@@ -57,6 +54,7 @@
from lp.testing import (
ANONYMOUS,
login,
+ login_celebrity,
login_person,
person_logged_in,
StormStatementRecorder,
@@ -136,7 +134,7 @@
def test_private_superteams_anonymous(self):
# If the viewer is anonymous, the portlet is not shown.
team = self.createTeams()
- viewer = self.factory.makePerson()
+ self.factory.makePerson()
view = create_initialized_view(
team, '+index', server_url=canonical_url(team), path_info='')
html = view()
@@ -324,18 +322,7 @@
self.failUnless(person_view.should_show_ppa_section)
-class TestPersonEditView(TestCaseWithFactory):
-
- layer = LaunchpadFunctionalLayer
-
- def setUp(self):
- TestCaseWithFactory.setUp(self)
- self.valid_email_address = self.factory.getUniqueEmailAddress()
- self.person = self.factory.makePerson(email=self.valid_email_address)
- login_person(self.person)
- self.ppa = self.factory.makeArchive(owner=self.person)
- self.view = PersonEditView(
- self.person, LaunchpadTestRequest())
+class TestPersonRenameFormMixin:
def test_can_rename_with_empty_PPA(self):
# If a PPA exists but has no packages, we can rename the
@@ -380,6 +367,19 @@
self.view.initialize()
self.assertFalse(self.view.form_fields['name'].for_display)
+
+class TestPersonEditView(TestPersonRenameFormMixin, TestCaseWithFactory):
+
+ layer = LaunchpadFunctionalLayer
+
+ def setUp(self):
+ super(TestPersonEditView, self).setUp()
+ self.valid_email_address = self.factory.getUniqueEmailAddress()
+ self.person = self.factory.makePerson(email=self.valid_email_address)
+ login_person(self.person)
+ self.ppa = self.factory.makeArchive(owner=self.person)
+ self.view = create_initialized_view(self.person, '+edit')
+
def test_add_email_good_data(self):
email_address = self.factory.getUniqueEmailAddress()
form = {
@@ -423,6 +423,34 @@
self.assertEqual(expected_msg, error_msg)
+class PersonAdministerViewTestCase(TestPersonRenameFormMixin,
+ TestCaseWithFactory):
+ layer = LaunchpadFunctionalLayer
+
+ def setUp(self):
+ super(PersonAdministerViewTestCase, self).setUp()
+ self.person = self.factory.makePerson()
+ login_celebrity('admin')
+ self.ppa = self.factory.makeArchive(owner=self.person)
+ self.view = create_initialized_view(self.person, '+review')
+
+ def test_init_admin(self):
+ # An admin sees all the fields.
+ self.assertEqual('Review person', self.view.label)
+ self.assertEqual(
+ ['name', 'displayname', 'personal_standing',
+ 'personal_standing_reason'],
+ self.view.field_names)
+
+ def test_init_registry_expert(self):
+ # Registry experts do no see the the displayname field.
+ login_celebrity('registry_experts')
+ self.view.setUpFields()
+ self.assertEqual(
+ ['name', 'personal_standing', 'personal_standing_reason'],
+ self.view.field_names)
+
+
class TestTeamCreationView(TestCaseWithFactory):
layer = DatabaseFunctionalLayer
=== modified file 'lib/lp/registry/browser/tests/test_team.py'
--- lib/lp/registry/browser/tests/test_team.py 2012-01-23 18:15:34 +0000
+++ lib/lp/registry/browser/tests/test_team.py 2012-01-27 16:44:35 +0000
@@ -30,6 +30,7 @@
from lp.services.webapp.publisher import canonical_url
from lp.soyuz.enums import ArchiveStatus
from lp.testing import (
+ login_celebrity,
login_person,
person_logged_in,
TestCaseWithFactory,
@@ -173,9 +174,9 @@
self.acceptTeam(self.super_team, successful, failed)
-class TestTeamEditView(TestCaseWithFactory):
+class TestTeamPersonRenameFormMixin:
- layer = LaunchpadFunctionalLayer
+ view_name = None
def test_cannot_rename_team_with_active_ppa(self):
# A team with an active PPA that contains publications cannot be
@@ -186,7 +187,7 @@
self.factory.makeSourcePackagePublishingHistory(archive=archive)
get_property_cache(team).archive = archive
with person_logged_in(owner):
- view = create_initialized_view(team, name="+edit")
+ view = create_initialized_view(team, name=self.view_name)
self.assertTrue(view.form_fields['name'].for_display)
self.assertEqual(
'This team has an active PPA with packages published and '
@@ -201,7 +202,7 @@
removeSecurityProxy(archive).status = ArchiveStatus.DELETED
get_property_cache(team).archive = archive
with person_logged_in(owner):
- view = create_initialized_view(team, name="+edit")
+ view = create_initialized_view(team, name=self.view_name)
self.assertFalse(view.form_fields['name'].for_display)
def test_cannot_rename_team_with_active_mailinglist(self):
@@ -211,7 +212,7 @@
team = self.factory.makeTeam(owner=owner)
self.factory.makeMailingList(team, owner)
with person_logged_in(owner):
- view = create_initialized_view(team, name="+edit")
+ view = create_initialized_view(team, name=self.view_name)
self.assertTrue(view.form_fields['name'].for_display)
self.assertEqual(
'This team has a mailing list and may not be renamed.',
@@ -226,7 +227,7 @@
team_list.transitionToStatus(MailingListStatus.INACTIVE)
team_list.purge()
with person_logged_in(owner):
- view = create_initialized_view(team, name="+edit")
+ view = create_initialized_view(team, name=self.view_name)
self.assertFalse(view.form_fields['name'].for_display)
def test_cannot_rename_team_with_multiple_reasons(self):
@@ -240,13 +241,19 @@
self.factory.makeSourcePackagePublishingHistory(archive=archive)
get_property_cache(team).archive = archive
with person_logged_in(owner):
- view = create_initialized_view(team, name="+edit")
+ view = create_initialized_view(team, name=self.view_name)
self.assertTrue(view.form_fields['name'].for_display)
self.assertEqual(
'This team has an active PPA with packages published and '
'a mailing list and may not be renamed.',
view.widgets['name'].hint)
+
+class TestTeamEditView(TestTeamPersonRenameFormMixin, TestCaseWithFactory):
+
+ layer = LaunchpadFunctionalLayer
+ view_name = '+edit'
+
def test_edit_team_view_permission(self):
# Only an administrator or the team owner of a team can
# change the details of that team.
@@ -437,6 +444,29 @@
view.errors[0].doc())
+class TeamAdminisiterViewTestCase(TestTeamPersonRenameFormMixin,
+ TestCaseWithFactory):
+
+ layer = LaunchpadFunctionalLayer
+ view_name = '+review'
+
+ def test_init_admin(self):
+ # An admin sees all the fields.
+ team = self.factory.makeTeam()
+ login_celebrity('admin')
+ view = create_initialized_view(team, name=self.view_name)
+ self.assertEqual('Review team', view.label)
+ self.assertEqual(
+ ['name', 'displayname'], view.field_names)
+
+ def test_init_registry_expert(self):
+ # Registry experts do no see the the displayname field.
+ team = self.factory.makeTeam()
+ login_celebrity('registry_experts')
+ view = create_initialized_view(team, name=self.view_name)
+ self.assertEqual(['name'], view.field_names)
+
+
class TestTeamMenu(TestCaseWithFactory):
layer = DatabaseFunctionalLayer
