Merge lp:~sinzui/launchpad/add-private-member into lp:launchpad

Proposed by Curtis Hovey
Status: Merged
Approved by: j.c.sackett
Approved revision: no longer in the source branch.
Merged at revision: 15641
Proposed branch: lp:~sinzui/launchpad/add-private-member
Merge into: lp:launchpad
Diff against target: 41 lines (+11/-2)
2 files modified
lib/lp/registry/browser/team.py (+2/-2)
lib/lp/registry/browser/tests/test_team.py (+9/-0)
To merge this branch: bzr merge lp:~sinzui/launchpad/add-private-member
Reviewer Review Type Date Requested Status
j.c.sackett (community) Approve
Review via email: mp+115419@code.launchpad.net

Commit message

+addmember permits private teams.

Description of the change

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

To post a comment you must log in.
Revision history for this message
j.c.sackett (jcsackett) wrote :

Awesome, glad it was as simple as we had supposed. Thanks.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/registry/browser/team.py'
2--- lib/lp/registry/browser/team.py 2012-07-12 00:46:07 +0000
3+++ lib/lp/registry/browser/team.py 2012-07-17 19:12:33 +0000
4@@ -143,7 +143,7 @@
5 from lp.security import ModerateByRegistryExpertsOrAdmins
6 from lp.services.config import config
7 from lp.services.features import getFeatureFlag
8-from lp.services.fields import PublicPersonChoice
9+from lp.services.fields import PersonChoice
10 from lp.services.identity.interfaces.emailaddress import IEmailAddressSet
11 from lp.services.privacy.interfaces import IObjectPrivacy
12 from lp.services.propertycache import cachedproperty
13@@ -1141,7 +1141,7 @@
14 class ITeamMember(Interface):
15 """The interface used in the form to add a new member to a team."""
16
17- newmember = PublicPersonChoice(
18+ newmember = PersonChoice(
19 title=_('New member'), required=True,
20 vocabulary='ValidTeamMember',
21 description=_("The user or team which is going to be "
22
23=== modified file 'lib/lp/registry/browser/tests/test_team.py'
24--- lib/lp/registry/browser/tests/test_team.py 2012-07-07 19:30:24 +0000
25+++ lib/lp/registry/browser/tests/test_team.py 2012-07-17 19:12:33 +0000
26@@ -742,6 +742,15 @@
27 self.assertEqual(
28 None, view.widgets['newmember']._getCurrentValue())
29
30+ def test_add_private_team_member_success(self):
31+ member = self.factory.makeTeam(
32+ name="a-member", owner=self.team.teamowner,
33+ visibility=PersonVisibility.PRIVATE)
34+ form = self.getForm(member)
35+ view = create_initialized_view(self.team, "+addmember", form=form)
36+ self.assertEqual([], view.errors)
37+ self.assertTrue(member.inTeam(self.team))
38+
39 def test_add_former_member_success(self):
40 member = self.factory.makePerson(name="a-member")
41 self.team.addMember(member, self.team.teamowner)