Merge lp:~allenap/launchpad/prune-bug-notification-recipients into lp:launchpad

[Gavin Panella]

[r=mars][ui=none][no-qa] Remove a couple of unused methods in BugNotificationRecipients.

Remove a couple of unused methods in BugNotificationRecipients.

I have a lint fix for the doctest. I'll land that with the branch but I haven't pushed it yet so that the diff isn't noisy.

[Māris Fogels (mars)]

Hi Gavin,

This looks good. I am glad to see the API surface shrink a bit. +1

Maris

review: Approve

=== modified file 'lib/lp/bugs/doc/bugnotificationrecipients.txt'
--- lib/lp/bugs/doc/bugnotificationrecipients.txt	2010-10-18 22:24:59 +0000
+++ lib/lp/bugs/doc/bugnotificationrecipients.txt	2010-12-07 17:03:30 +0000
@@ -4,55 +4,62 @@
 the INotificationRecipientSet interface for bug notifications. (See
 notification-recipient-set.txt for the details.)
 
-== How it's used ==
+How it's used
+=============
 
 The IBug.getBugNotificationRecipients() implementation creates and uses
 BugNotificationRecipients instances. Here's an example of this in
 action:
 
-  >>> from lp.bugs.model.bug import Bug
-  >>> from lp.registry.model.distribution import Distribution
-  >>> from lp.registry.model.product import Product
-  >>> bug_one = Bug.get(1)
-  >>> recipients = bug_one.getBugNotificationRecipients()
+    >>> from lp.bugs.model.bug import Bug
+    >>> from lp.registry.model.distribution import Distribution
+    >>> from lp.registry.model.product import Product
+    >>> bug_one = Bug.get(1)
+    >>> recipients = bug_one.getBugNotificationRecipients()
 
 The instance of BugNotificationRecipients we get back correctly
 implements INotificationRecipientSet:
 
-  >>> from canonical.launchpad.interfaces.launchpad import INotificationRecipientSet
-  >>> from canonical.launchpad.webapp.testing import verifyObject
-  >>> verifyObject(INotificationRecipientSet, recipients)
-  True
+    >>> from canonical.launchpad.interfaces.launchpad import (
+    ...     INotificationRecipientSet,
+    ...     )
+    >>> from canonical.launchpad.webapp.testing import verifyObject
+    >>> verifyObject(INotificationRecipientSet, recipients)
+    True
 
 This instance contains email addresses and rationales. Let's define a
 helper function so we can format this output:
 
-  >>> def print_rationales(rationale):
-  ...   for address in rationale.getEmails():
-  ...     text, header = rationale.getReason(address)
-  ...     print address
-  ...     print "    %s" % header
-  ...     print "    %s" % text
+    >>> def print_rationales(rationale):
+    ...   for address in rationale.getEmails():
+    ...     text, header = rationale.getReason(address)
+    ...     print address
+    ...     print "    %s" % header
+    ...     print "    %s" % text
 
 And print them out. The first line is the email address; second is the
 text appropriate to be used in an X- header, and the last is the text
 appropriate for an email footer.
 
-  >>> print_rationales(recipients)
-  [e-mail]
-      Subscriber (mozilla-firefox in ubuntu)
-      You received this bug notification because you are subscribed to mozilla-firefox in ubuntu.
-  [e-mail]
-      Assignee
-      You received this bug notification because you are a bug assignee.
-  [e-mail]
-      Registrant (Ubuntu) @ubuntu-team
-      You received this bug notification because you are a member of Ubuntu Team, which is the registrant for Ubuntu.
-  [e-mail]
-      Subscriber
-      You received this bug notification because you are a direct subscriber of the bug.
+    >>> print_rationales(recipients)
+    [e-mail]
+        Subscriber (mozilla-firefox in ubuntu)
+        You received this bug notification because you are subscribed
+        to mozilla-firefox in ubuntu.
+    [e-mail]
+        Assignee
+        You received this bug notification because you are a bug assignee.
+    [e-mail]
+        Registrant (Ubuntu) @ubuntu-team
+        You received this bug notification because you are a member of
+        Ubuntu Team, which is the registrant for Ubuntu.
+    [e-mail]
+        Subscriber
+        You received this bug notification because you are a direct
+        subscriber of the bug.
 
-== The Bug-BugNotificationRecipients API ==
+The Bug-BugNotificationRecipients API
+=====================================
 
 Most of the API of BugNotificationRecipients is actually kept private
 between the Bug class and itself. Let's now demonstrate the API that Bug
@@ -62,125 +69,111 @@
 
 Let's up some data for our test:
 
-  >>> from lp.registry.interfaces.person import IPersonSet
-  >>> from lp.bugs.mail.bugnotificationrecipients import (
-  ...     BugNotificationRecipients)
-  >>> debian = Distribution.selectOneBy(name="debian")
-  >>> pmount = debian.getSourcePackage("pmount")
-  >>> alsa_utils = Product.selectOneBy(name="alsa-utils")
-  >>> gnomebaker = Product.selectOneBy(name="gnomebaker")
-  >>> personset = getUtility(IPersonSet)
+    >>> from lp.registry.interfaces.person import IPersonSet
+    >>> from lp.bugs.mail.bugnotificationrecipients import (
+    ...     BugNotificationRecipients)
+    >>> debian = Distribution.selectOneBy(name="debian")
+    >>> pmount = debian.getSourcePackage("pmount")
+    >>> alsa_utils = Product.selectOneBy(name="alsa-utils")
+    >>> gnomebaker = Product.selectOneBy(name="gnomebaker")
+    >>> personset = getUtility(IPersonSet)
 
 Here's where getBugNotificationRecipients() starts off. First, a
 BugNotificationRecipients instance is created:
 
-  >>> recipients = BugNotificationRecipients()
+    >>> recipients = BugNotificationRecipients()
 
 Then, subscribers of various types are added:
 
-  >>> foo_bar = personset.getByEmail("[e-mail]")
-  >>> recipients.addDupeSubscriber(foo_bar)
-
-  >>> test = personset.getByEmail("[e-mail]")
-  >>> recipients.addDirectSubscriber(test)
-
-  >>> no_priv = personset.getByEmail("[e-mail]")
-  >>> recipients.addAssignee(no_priv)
-
-  >>> daf = personset.getByEmail("[e-mail]")
-  >>> recipients.addDistroBugSupervisor(daf, debian)
-
-  >>> carlos = personset.getByEmail("[e-mail]")
-  >>> recipients.addStructuralSubscriber(carlos, pmount)
-
-  >>> marilize = personset.getByEmail("[e-mail]")
-  >>> recipients.addUpstreamBugSupervisor(marilize, alsa_utils)
-
-  >>> testtest = personset.getByEmail("[e-mail]")
-  >>> recipients.addRegistrant(testtest, gnomebaker)
+    >>> foo_bar = personset.getByEmail("[e-mail]")
+    >>> recipients.addDupeSubscriber(foo_bar)
+
+    >>> test = personset.getByEmail("[e-mail]")
+    >>> recipients.addDirectSubscriber(test)
+
+    >>> no_priv = personset.getByEmail("[e-mail]")
+    >>> recipients.addAssignee(no_priv)
+
+    >>> carlos = personset.getByEmail("[e-mail]")
+    >>> recipients.addStructuralSubscriber(carlos, pmount)
+
+    >>> testtest = personset.getByEmail("[e-mail]")
+    >>> recipients.addRegistrant(testtest, gnomebaker)
 
 If we print out the recipients and rationales, here's what we get:
 
-  >>> print_rationales(recipients)
-  [e-mail]
-      Subscriber (pmount in debian)
-      You received this bug notification because you are subscribed to pmount in debian.
-  [e-mail]
-      Bug Supervisor (Debian)
-      You received this bug notification because you are the bug supervisor for Debian.
-  [e-mail]
-      Subscriber of Duplicate
-      You received this bug notification because you are a direct subscriber of a duplicate bug.
-  [e-mail]
-      Bug Supervisor (alsa-utils)
-      You received this bug notification because you are the bug supervisor for alsa-utils.
-  [e-mail]
-      Assignee
-      You received this bug notification because you are a bug assignee.
-  [e-mail]
-      Subscriber
-      You received this bug notification because you are a direct subscriber of the bug.
+    >>> print_rationales(recipients)
+    [e-mail]
+        Subscriber (pmount in debian)
+        You received this bug notification because you are subscribed
+        to pmount in debian.
+    [e-mail]
+        Subscriber of Duplicate
+        You received this bug notification because you are a direct
+        subscriber of a duplicate bug.
+    [e-mail]
+        Assignee
+        You received this bug notification because you are a bug
+        assignee.
+    [e-mail]
+        Subscriber
+        You received this bug notification because you are a direct
+        subscriber of the bug.
 
 Note how we account for every important variation in bug subscriptions
 here: bug supervisors, subscribers, dupe subscribers and more.
 
-== A duplicate bug modification notifies its main bug ==
+A duplicate bug modification notifies its main bug
+==================================================
 
 If the bug we are changing is actually a duplicate of another bug, an
 additional step is involved. A BugNotificationRecipients instance is
 created, annotating that it represents a master bug (of which we are a
 duplicate of).
 
-  >>> bug_two = Bug.get(2)
-  >>> recipients = BugNotificationRecipients(duplicateof=bug_two)
-
-  >>> foo_bar = personset.getByEmail("[e-mail]")
-  >>> recipients.addDupeSubscriber(foo_bar)
-
-  >>> test = personset.getByEmail("[e-mail]")
-  >>> recipients.addDirectSubscriber(test)
-
-  >>> no_priv = personset.getByEmail("[e-mail]")
-  >>> recipients.addAssignee(no_priv)
-
-  >>> daf = personset.getByEmail("[e-mail]")
-  >>> recipients.addDistroBugSupervisor(daf, debian)
-
-  >>> carlos = personset.getByEmail("[e-mail]")
-  >>> recipients.addStructuralSubscriber(carlos, pmount)
-
-  >>> marilize = personset.getByEmail("[e-mail]")
-  >>> recipients.addUpstreamBugSupervisor(marilize, alsa_utils)
-
-  >>> testtest = personset.getByEmail("[e-mail]")
-  >>> recipients.addRegistrant(testtest, gnomebaker)
+    >>> bug_two = Bug.get(2)
+    >>> recipients = BugNotificationRecipients(duplicateof=bug_two)
+
+    >>> foo_bar = personset.getByEmail("[e-mail]")
+    >>> recipients.addDupeSubscriber(foo_bar)
+
+    >>> test = personset.getByEmail("[e-mail]")
+    >>> recipients.addDirectSubscriber(test)
+
+    >>> no_priv = personset.getByEmail("[e-mail]")
+    >>> recipients.addAssignee(no_priv)
+
+    >>> carlos = personset.getByEmail("[e-mail]")
+    >>> recipients.addStructuralSubscriber(carlos, pmount)
+
+    >>> testtest = personset.getByEmail("[e-mail]")
+    >>> recipients.addRegistrant(testtest, gnomebaker)
 
 If you print out rationales in this situation, you'll see that the
 message says "via Bug 2". The reason for this is that the people being
 notified here are actually subscribed to bug 2, and they may be asking
 themselves why the hell they are getting email for bug 1.
 
-  >>> print_rationales(recipients)
-  [e-mail]
-      Subscriber (pmount in debian) via Bug 2
-      You received this bug notification because you are subscribed to pmount in debian (via bug 2).
-  [e-mail]
-      Bug Supervisor (Debian) via Bug 2
-      You received this bug notification because you are the bug supervisor for Debian (via bug 2).
-  [e-mail]
-      Subscriber of Duplicate via Bug 2
-      You received this bug notification because you are a direct subscriber of a duplicate bug (via bug 2).
-  [e-mail]
-      Bug Supervisor (alsa-utils) via Bug 2
-      You received this bug notification because you are the bug supervisor for alsa-utils (via bug 2).
-  [e-mail]
-      Assignee via Bug 2
-      You received this bug notification because you are a bug assignee (via bug 2).
-  [e-mail]
-      Subscriber via Bug 2
-      You received this bug notification because you are a direct subscriber of the bug (via bug 2).
+    >>> print_rationales(recipients)
+    [e-mail]
+        Subscriber (pmount in debian) via Bug 2
+        You received this bug notification because you are subscribed
+        to pmount in debian (via bug 2).
+    [e-mail]
+        Subscriber of Duplicate via Bug 2
+        You received this bug notification because you are a direct
+        subscriber of a duplicate bug (via bug 2).
+    [e-mail]
+        Assignee via Bug 2
+        You received this bug notification because you are a bug
+        assignee (via bug 2).
+    [e-mail]
+        Subscriber via Bug 2
+        You received this bug notification because you are a direct
+        subscriber of the bug (via bug 2).
 
-== Team subscribers are special ==
+Team subscribers are special
+============================
 
 In the case where the teams are subscribers, things vary according to
 whether the team has a contact email address or not. When there is no
@@ -190,71 +183,72 @@
 
 Here's an example of this situation:
 
-  >>> recipients = BugNotificationRecipients()
-  >>> testing_spanish_team = personset.getByName("testing-spanish-team")
-  >>> recipients.addDupeSubscriber(testing_spanish_team)
-
-  >>> guadamen = personset.getByName("guadamen")
-  >>> recipients.addAssignee(guadamen)
-
-  >>> name20 = personset.getByName("name20")
-  >>> recipients.addStructuralSubscriber(name20, pmount)
-
-  >>> name18 = personset.getByName("name18")
-  >>> recipients.addUpstreamBugSupervisor(name18, alsa_utils)
-
-  >>> vcs_imports = personset.getByName("vcs-imports")
-  >>> recipients.addRegistrant(vcs_imports, gnomebaker)
-
-  >>> shipit_admins = personset.getByName("shipit-admins")
-  >>> recipients.addDirectSubscriber(shipit_admins)
-
-  >>> launchpad_buildd_admins = personset.getByName("launchpad-buildd-admins")
-  >>> recipients.addDistroBugSupervisor(launchpad_buildd_admins, debian)
+    >>> recipients = BugNotificationRecipients()
+    >>> testing_spanish_team = personset.getByName("testing-spanish-team")
+    >>> recipients.addDupeSubscriber(testing_spanish_team)
+
+    >>> guadamen = personset.getByName("guadamen")
+    >>> recipients.addAssignee(guadamen)
+
+    >>> name20 = personset.getByName("name20")
+    >>> recipients.addStructuralSubscriber(name20, pmount)
+
+    >>> vcs_imports = personset.getByName("vcs-imports")
+    >>> recipients.addRegistrant(vcs_imports, gnomebaker)
+
+    >>> shipit_admins = personset.getByName("shipit-admins")
+    >>> recipients.addDirectSubscriber(shipit_admins)
 
 You'll notice that the rationales this time state clearly which team
 membership is causing us to send mail.
 
-  >>> print_rationales(recipients)
-    [e-mail]
-        Bug Supervisor (alsa-utils) @name18
-        You received this bug notification because you are a member of Ubuntu Gnome Team, which is the bug supervisor for alsa-utils.
-    [e-mail]
-        Subscriber of Duplicate @testing-spanish-team
-        You received this bug notification because you are a member of testing Spanish team, which is a subscriber of a duplicate bug.
-    [e-mail]
-        Bug Supervisor (Debian) @launchpad-buildd-admins
-        You received this bug notification because you are a member of Launchpad Buildd Admins, which is the bug supervisor for Debian.
-    [e-mail]
-        Registrant (gnomebaker) @vcs-imports
-        You received this bug notification because you are a member of VCS imports, which is the registrant for gnomebaker.
-    [e-mail]
-        Subscriber of Duplicate @testing-spanish-team
-        You received this bug notification because you are a member of testing Spanish team, which is a subscriber of a duplicate bug.
-    [e-mail]
-        Subscriber of Duplicate @testing-spanish-team
-        You received this bug notification because you are a member of testing Spanish team, which is a subscriber of a duplicate bug.
-    [e-mail]
-        Bug Supervisor (alsa-utils) @name18
-        You received this bug notification because you are a member of Ubuntu Gnome Team, which is the bug supervisor for alsa-utils.
-    [e-mail]
-        Subscriber @shipit-admins
-        You received this bug notification because you are a member of ShipIt Administrators, which is a direct subscriber.
-    [e-mail]
-        Subscriber of Duplicate @testing-spanish-team
-        You received this bug notification because you are a member of testing Spanish team, which is a subscriber of a duplicate bug.
-    [e-mail]
-        Registrant (gnomebaker) @vcs-imports
-        You received this bug notification because you are a member of VCS imports, which is the registrant for gnomebaker.
-    [e-mail]
-        Assignee @guadamen
-        You received this bug notification because you are a member of GuadaMen, which is a bug assignee.
-    [e-mail]
-        Subscriber (pmount in debian) @name20
-        You received this bug notification because you are a member of Warty Security Team, which is subscribed to pmount in debian.
-    [e-mail]
-        Subscriber of Duplicate @testing-spanish-team
-        You received this bug notification because you are a member of testing Spanish team, which is a subscriber of a duplicate bug.
+    >>> print_rationales(recipients)
+      [e-mail]
+          Subscriber of Duplicate @testing-spanish-team
+          You received this bug notification because you are a member
+          of testing Spanish team, which is a subscriber of a
+          duplicate bug.
+      [e-mail]
+          Registrant (gnomebaker) @vcs-imports
+          You received this bug notification because you are a member
+          of VCS imports, which is the registrant for gnomebaker.
+      [e-mail]
+          Subscriber of Duplicate @testing-spanish-team
+          You received this bug notification because you are a member
+          of testing Spanish team, which is a subscriber of a
+          duplicate bug.
+      [e-mail]
+          Subscriber of Duplicate @testing-spanish-team
+          You received this bug notification because you are a member
+          of testing Spanish team, which is a subscriber of a
+          duplicate bug.
+      [e-mail]
+          Subscriber @shipit-admins
+          You received this bug notification because you are a member
+          of ShipIt Administrators, which is a direct subscriber.
+      [e-mail]
+          Subscriber of Duplicate @testing-spanish-team
+          You received this bug notification because you are a member
+          of testing Spanish team, which is a subscriber of a
+          duplicate bug.
+      [e-mail]
+          Registrant (gnomebaker) @vcs-imports
+          You received this bug notification because you are a member
+          of VCS imports, which is the registrant for gnomebaker.
+      [e-mail]
+          Assignee @guadamen
+          You received this bug notification because you are a member
+          of GuadaMen, which is a bug assignee.
+      [e-mail]
+          Subscriber (pmount in debian) @name20
+          You received this bug notification because you are a member
+          of Warty Security Team, which is subscribed to pmount in
+          debian.
+      [e-mail]
+          Subscriber of Duplicate @testing-spanish-team
+          You received this bug notification because you are a member
+          of testing Spanish team, which is a subscriber of a
+          duplicate bug.
 
 This doesn't help the end-user too much if he's a member of this team
 indirectly (for instance, if he's a member of a team which is in turn a
@@ -262,47 +256,52 @@
 the team page and see the membership graph directly. This may be worth
 fixing in the future.
 
-== First impressions stick ==
+First impressions stick
+=======================
 
 Another important property of BugNotificationRecipients is that the
 first rationale presented to it is the one that is presented -- even if
 the recipient has multiple reasons for which he might be emailed. Here's
 a pathological example:
 
-  >>> recipients = BugNotificationRecipients()
-  >>> recipients.addDirectSubscriber(test)
-  >>> recipients.addAssignee(test)
-  >>> recipients.addRegistrant(test, gnomebaker)
-  >>> recipients.addDirectSubscriber(foo_bar)
+    >>> recipients = BugNotificationRecipients()
+    >>> recipients.addDirectSubscriber(test)
+    >>> recipients.addAssignee(test)
+    >>> recipients.addRegistrant(test, gnomebaker)
+    >>> recipients.addDirectSubscriber(foo_bar)
 
 This guy is emailed because he's a direct subscriber, an assignee and an
 upstream registrant. However, if we ask the rationales instance:
 
-  >>> print_rationales(recipients)
-  [e-mail]
-      Subscriber
-      You received this bug notification because you are a direct subscriber of the bug.
-  [e-mail]
-      Subscriber
-      You received this bug notification because you are a direct subscriber of the bug.
+    >>> print_rationales(recipients)
+    [e-mail]
+        Subscriber
+        You received this bug notification because you are a direct
+        subscriber of the bug.
+    [e-mail]
+        Subscriber
+        You received this bug notification because you are a direct
+        subscriber of the bug.
 
 Only the first rationale is presented. This is the case even if we
 update this set of recipients with another one:
 
-  >>> recipients2 = BugNotificationRecipients()
-  >>> recipients2.addDupeSubscriber(test)
-  >>> recipients2.update(recipients)
+    >>> recipients2 = BugNotificationRecipients()
+    >>> recipients2.addDupeSubscriber(test)
+    >>> recipients2.update(recipients)
 
 The rationales for [e-mail] in the 'recipients' instance just
 don't matter:
 
-  >>> print_rationales(recipients2)
-  [e-mail]
-      Subscriber
-      You received this bug notification because you are a direct subscriber of the bug.
-  [e-mail]
-      Subscriber of Duplicate
-      You received this bug notification because you are a direct subscriber of a duplicate bug.
+    >>> print_rationales(recipients2)
+    [e-mail]
+        Subscriber
+        You received this bug notification because you are a direct
+        subscriber of the bug.
+    [e-mail]
+        Subscriber of Duplicate
+        You received this bug notification because you are a direct
+        subscriber of a duplicate bug.
 
 This may be seen as a limitation, but you don't want a 10-line rationale
 footer for people who are central to Launchpad, so for now it's the way

=== modified file 'lib/lp/bugs/mail/bugnotificationrecipients.py'
--- lib/lp/bugs/mail/bugnotificationrecipients.py	2010-10-23 16:25:51 +0000
+++ lib/lp/bugs/mail/bugnotificationrecipients.py	2010-12-07 17:03:30 +0000
@@ -207,21 +207,6 @@
             text = "are a bug assignee"
         self._addReason(person, text, reason)
 
-    def addDistroBugSupervisor(self, person, distro):
-        """Registers a distribution bug supervisor for this bug."""
-        reason = "Bug Supervisor (%s)" % distro.displayname
-        # All displaynames in these reasons should be changed to bugtargetname
-        # (as part of bug 113262) once bugtargetname is finalized for packages
-        # (bug 113258). Changing it before then would be excessively
-        # disruptive.
-        if person.isTeam():
-            text = ("are a member of %s, which is the bug supervisor for %s" %
-                (person.displayname, distro.displayname))
-            reason += " @%s" % person.name
-        else:
-            text = "are the bug supervisor for %s" % distro.displayname
-        self._addReason(person, text, reason)
-
     def addStructuralSubscriber(self, person, target):
         """Registers a structural subscriber to this bug's target."""
         reason = "Subscriber (%s)" % target.displayname
@@ -233,17 +218,6 @@
             text = "are subscribed to %s" % target.displayname
         self._addReason(person, text, reason)
 
-    def addUpstreamBugSupervisor(self, person, upstream):
-        """Registers an upstream bug supervisor for this bug."""
-        reason = "Bug Supervisor (%s)" % upstream.displayname
-        if person.isTeam():
-            text = ("are a member of %s, which is the bug supervisor for %s" %
-                (person.displayname, upstream.displayname))
-            reason += " @%s" % person.name
-        else:
-            text = "are the bug supervisor for %s" % upstream.displayname
-        self._addReason(person, text, reason)
-
     def addRegistrant(self, person, upstream):
         """Registers an upstream product registrant for this bug."""
         reason = "Registrant (%s)" % upstream.displayname