launchpad-reviewers team mailing list archive

[Curtis Hovey <curtis.hovey@xxxxxxxxxxxxx>]

Curtis Hovey has proposed merging lp:~sinzui/launchpad/add-private-member into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~sinzui/launchpad/add-private-member/+merge/115419

Private teams can be added to a team using the "Add team member" picker, but
+addmember gives an error.

--------------------------------------------------------------------

RULES

    Pre-implementation: jcsackett
    * This looks like a vocab issue. What vocab do the picker and form use?
      Does the view have a secondary validation that rejects private teams.
      * The view uses the ITeamMember schema which wrongly uses
        PublicPersonChoice with ValidTeamMember. ValidTeamMember does to
        proper validation, but the PublicPersonChoice field adds an unwanted
        public team check.
      * The field should be PersonChoice as is used in ITeamMembership.person.


QA

    * https://qastaging.launchpad.net/~gdp-developers/+addmember
    * Add private-registry-test-team
    * Verify the team was accepted


LINT

    lib/lp/registry/browser/team.py
    lib/lp/registry/browser/tests/test_team.py


TEST

    ./bin/test -vvc -t TestTeamMemberAdd lp.registry.browser.tests.test_team


IMPLEMENTATION

Replace PublicPersonChoice with PersonChoice so that the vocabulary does
all the validation.
    lib/lp/registry/browser/team.py
    lib/lp/registry/browser/tests/test_team.py

-- 
https://code.launchpad.net/~sinzui/launchpad/add-private-member/+merge/115419
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~sinzui/launchpad/add-private-member into lp:launchpad.

=== modified file 'lib/lp/registry/browser/team.py'
--- lib/lp/registry/browser/team.py	2012-07-12 00:46:07 +0000
+++ lib/lp/registry/browser/team.py	2012-07-17 19:12:33 +0000
@@ -143,7 +143,7 @@
 from lp.security import ModerateByRegistryExpertsOrAdmins
 from lp.services.config import config
 from lp.services.features import getFeatureFlag
-from lp.services.fields import PublicPersonChoice
+from lp.services.fields import PersonChoice
 from lp.services.identity.interfaces.emailaddress import IEmailAddressSet
 from lp.services.privacy.interfaces import IObjectPrivacy
 from lp.services.propertycache import cachedproperty
@@ -1141,7 +1141,7 @@
 class ITeamMember(Interface):
     """The interface used in the form to add a new member to a team."""
 
-    newmember = PublicPersonChoice(
+    newmember = PersonChoice(
         title=_('New member'), required=True,
         vocabulary='ValidTeamMember',
         description=_("The user or team which is going to be "

=== modified file 'lib/lp/registry/browser/tests/test_team.py'
--- lib/lp/registry/browser/tests/test_team.py	2012-07-07 19:30:24 +0000
+++ lib/lp/registry/browser/tests/test_team.py	2012-07-17 19:12:33 +0000
@@ -742,6 +742,15 @@
         self.assertEqual(
             None, view.widgets['newmember']._getCurrentValue())
 
+    def test_add_private_team_member_success(self):
+        member = self.factory.makeTeam(
+            name="a-member", owner=self.team.teamowner,
+            visibility=PersonVisibility.PRIVATE)
+        form = self.getForm(member)
+        view = create_initialized_view(self.team, "+addmember", form=form)
+        self.assertEqual([], view.errors)
+        self.assertTrue(member.inTeam(self.team))
+
     def test_add_former_member_success(self):
         member = self.factory.makePerson(name="a-member")
         self.team.addMember(member, self.team.teamowner)