launchpad-reviewers team mailing list archive

[Robert Collins <robertc@xxxxxxxxxxxxxxxxx>]

Robert Collins has proposed merging lp:~lifeless/launchpad/bug-815623 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #815623 in Launchpad itself: "Mail notifications sent to team admins on joins / leaves to open teams"
  https://bugs.launchpad.net/launchpad/+bug/815623

For more details, see:
https://code.launchpad.net/~lifeless/launchpad/bug-815623/+merge/69021

Do not notify team admins about membership changes in open teams. I filed the linked bug about this because its getting rather noisy the more mailing lists I admin in LP :).

However, this might be contentious, so I could look at feature flagging it up - but, we don't have a good story for flags-in-jobs yet, so I'd rather just do this, I think its a very much in-spirit-with-our-less-mail-thing to make this change.
-- 
https://code.launchpad.net/~lifeless/launchpad/bug-815623/+merge/69021
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~lifeless/launchpad/bug-815623 into lp:launchpad.

=== modified file 'lib/lp/registry/doc/teammembership-email-notification.txt'
--- lib/lp/registry/doc/teammembership-email-notification.txt	2011-07-01 19:17:03 +0000
+++ lib/lp/registry/doc/teammembership-email-notification.txt	2011-07-25 02:52:31 +0000
@@ -30,6 +30,7 @@
     >>> from lp.registry.interfaces.person import (
     ...     IPersonSet,
     ...     TeamMembershipRenewalPolicy,
+    ...     TeamSubscriptionPolicy,
     ...     )
     >>> from lp.registry.interfaces.teammembership import (
     ...     ITeamMembershipSet,
@@ -45,6 +46,31 @@
     >>> admin_person = personset.getByEmail(ADMIN_EMAIL)
 
 
+In open teams joining and leaving the team generates no notifications.
+
+    >>> login_person(admin_person)
+    >>> ubuntu_team_policy = ubuntu_team.subscriptionpolicy
+    >>> ubuntu_team.subscriptionpolicy = TeamSubscriptionPolicy.OPEN
+    >>> base_mails = len(stub.test_emails)
+    >>> new_person = factory.makePerson()
+    >>> login_person(new_person)
+    >>> new_person.join(ubuntu_team)
+    >>> membership = membershipset.getByPersonAndTeam(new_person, ubuntu_team)
+    >>> membership.status.title
+    'Approved'
+    >>> run_mail_jobs()
+    >>> len(stub.test_emails) - base_mails
+    0
+    >>> new_person.leave(ubuntu_team)
+    >>> run_mail_jobs()
+    >>> len(stub.test_emails) - base_mails
+    0
+
+Put ubuntu back to the original status:
+
+    >>> login_person(admin_person)
+    >>> ubuntu_team.subscriptionpolicy = ubuntu_team_policy
+
 Now Robert Collins proposes himself as a member of the Ubuntu Team. This
 generates a notification email only to Ubuntu Team administrators.
 
@@ -875,8 +901,6 @@
     >>> ignored = team_one.addMember(member, owner)
     >>> print_distinct_emails()
     From: Team One ...
-    ----------------------------------------
-    From: Team One ...
     To: team-member...
     X-Launchpad-Message-Rationale: Member (team-one)
     Subject: You have been added to team-one
@@ -901,8 +925,6 @@
     >>> ignored = team_one.addMember(team_two, owner, force_team_add=True)
     >>> print_distinct_emails()
     From: Team One ...
-    ----------------------------------------
-    From: Team One ...
     To: team-two...
     X-Launchpad-Message-Rationale: Indirect member (team-one)
     Subject: team-two joined team-one

=== modified file 'lib/lp/registry/mail/notification.py'
--- lib/lp/registry/mail/notification.py	2011-05-12 21:33:10 +0000
+++ lib/lp/registry/mail/notification.py	2011-07-25 02:52:31 +0000
@@ -36,7 +36,10 @@
 from canonical.launchpad.webapp.publisher import canonical_url
 from canonical.launchpad.webapp.url import urlappend
 from lp.registry.interfaces.mailinglist import IHeldMessageDetails
-from lp.registry.interfaces.person import IPersonSet
+from lp.registry.interfaces.person import (
+    IPersonSet,
+    TeamSubscriptionPolicy,
+    )
 from lp.registry.interfaces.teammembership import (
     ITeamMembershipSet,
     TeamMembershipStatus,
@@ -177,6 +180,10 @@
     if not admin_addrs:
         return
 
+    # Open teams do not notify admins about new members.
+    if team.subscriptionpolicy == TeamSubscriptionPolicy.OPEN:
+        return
+
     replacements = {
         'person_name': "%s (%s)" % (person.displayname, person.name),
         'team_name': "%s (%s)" % (team.displayname, team.name),

=== modified file 'lib/lp/registry/model/persontransferjob.py'
--- lib/lp/registry/model/persontransferjob.py	2011-03-26 21:50:45 +0000
+++ lib/lp/registry/model/persontransferjob.py	2011-07-25 02:52:31 +0000
@@ -50,6 +50,7 @@
     IPerson,
     IPersonSet,
     ITeam,
+    TeamSubscriptionPolicy,
     )
 from lp.registry.interfaces.persontransferjob import (
     IMembershipNotificationJob,
@@ -296,7 +297,11 @@
             # Use the default template and subject.
             pass
 
-        if len(admin_emails) != 0:
+        # Must have someone to mail, and be a non-open team (because open teams
+        # are unrestricted, notifications on join/ leave do not help the
+        # admins.
+        if (len(admin_emails) != 0 and
+            self.team.subscriptionpolicy != TeamSubscriptionPolicy.OPEN):
             admin_template = get_email_template(
                 "%s-bulk.txt" % template_name, app='registry')
             for address in admin_emails: