Merge lp:~edwin-grubbs/launchpad/bug-498181-addmember-notify-oops into lp:launchpad

[Edwin Grubbs]

Fix oops when a user proposes another person as a member of a team via the REST API.

[Edwin Grubbs (edwin-grubbs)]

Summary
-------

In the UI, a user can only propose themself or a team they admin as a
member of another team. Some users of launchpadlib, started proposing
other people to keep track of their mentees. When the proposed member's
email addresses are hidden, this would result in an oops, since
addMember() would trigger an event that tries to email all the
admins of the team with the Reply-To header set to the email of
the proposed member. Since the member is being proposed by someone
else in the same way that teams are proposed, the Reply-To
will be set to the reviewer's email address.

Tests
-----

./bin/test -vv -t webservice/xx-person.txt

Demo and Q/A
------------

With launchpadlib, propose a member with hidden email addresses.

[Curtis Hovey (sinzui)]

This is a fine compromise betweent he API and Web interfaces. I have one grammatical suggestion.

=== modified file 'lib/canonical/launchpad/mailnotification.py'
--- lib/canonical/launchpad/mailnotification.py 2009-12-05 18:37:28 +0000
+++ lib/canonical/launchpad/mailnotification.py 2010-01-14 01:55:22 +0000
@@ -953,10 +953,13 @@
             'new-member-notification-for-admins.txt')
         subject = '%s joined %s' % (person.name, team.name)
     elif membership.status == proposed:
- if person.isTeam():
+ # In the UI, a user can only propose themself or a team they

grammar: s/themself/himself/

review: Approve (code)

=== renamed file 'lib/canonical/launchpad/emailtemplates/pending-membership-approval-for-teams.txt' => 'lib/canonical/launchpad/emailtemplates/pending-membership-approval-for-third-party.txt'
=== modified file 'lib/canonical/launchpad/mailnotification.py'
--- lib/canonical/launchpad/mailnotification.py	2009-12-05 18:37:28 +0000
+++ lib/canonical/launchpad/mailnotification.py	2010-01-14 23:01:13 +0000
@@ -953,10 +953,13 @@
             'new-member-notification-for-admins.txt')
         subject = '%s joined %s' % (person.name, team.name)
     elif membership.status == proposed:
-        if person.isTeam():
+        # In the UI, a user can only propose himself or a team he
+        # admins. Some users of the REST API have a workflow, where
+        # they propose users that are designated as mentees (Bug 498181).
+        if reviewer != person:
             headers = {"Reply-To": reviewer.preferredemail.email}
             template = get_email_template(
-                'pending-membership-approval-for-teams.txt')
+                'pending-membership-approval-for-third-party.txt')
         else:
             headers = {"Reply-To": person.preferredemail.email}
             template = get_email_template('pending-membership-approval.txt')

=== modified file 'lib/lp/registry/interfaces/person.py'
--- lib/lp/registry/interfaces/person.py	2010-01-07 06:27:46 +0000
+++ lib/lp/registry/interfaces/person.py	2010-01-14 23:01:13 +0000
@@ -1421,6 +1421,8 @@
 
         :param status: `TeamMembershipStatus` value must be either
             Approved, Proposed or Admin.
+            If the new member is a team, the status will be changed to
+            Invited unless the user is also an admin of that team.
 
         :param force_team_add: If the person is actually a team and
             force_team_add is False, the team will actually be invited to

=== modified file 'lib/lp/registry/stories/webservice/xx-person.txt'
--- lib/lp/registry/stories/webservice/xx-person.txt	2009-12-17 10:33:34 +0000
+++ lib/lp/registry/stories/webservice/xx-person.txt	2010-01-14 23:01:13 +0000
@@ -545,7 +545,34 @@
     ...     "/~landscape-developers/+member/salgado").jsonBody()['status']
     u'Deactivated'
 
-Adding an arbitrary member to a team:
+Though it is not possible through the Launchpad UI, some users of the
+REST API propose other people (as opposed to teams) as part of a
+mentoring process (Bug 498181).
+
+    >>> from canonical.launchpad.testing.pages import webservice_for_person
+    >>> from canonical.launchpad.webapp.interfaces import OAuthPermission
+    >>> from lp.registry.interfaces.person import IPersonSet
+    >>> from zope.component import getUtility
+    >>> login(ANONYMOUS)
+    >>> owner = getUtility(IPersonSet).getByName('owner')
+    >>> logout()
+    >>> owner_webservice = webservice_for_person(
+    ...     owner, permission=OAuthPermission.WRITE_PRIVATE)
+
+    # The sample user (name12) is used to verify that it works when
+    # the new member's email address is hidden.
+    >>> print owner_webservice.named_post(
+    ...     webservice.getAbsoluteUrl('~otherteam'), 'addMember', {},
+    ...     person=webservice.getAbsoluteUrl('/~name12'),
+    ...     status='Proposed', comment='Just a test')
+    HTTP/1.1 200 Ok
+    ...
+    >>> owner_webservice.get("/~otherteam/+member/name12"
+    ...     ).jsonBody()['status']
+    u'Proposed'
+
+Adding a team as a new member will result in the membership being
+set to the Invited status.
 
     >>> print webservice.named_post(
     ...     ubuntu_team['self_link'], 'addMember', {},