Merge lp:~allenap/launchpad/wire-up-filter-subs-bug-655567-devel into lp:launchpad

[Gavin Panella]

Merge wire-up-filter-subs-bug-655567 into devel.

Merge lp:~allenap/launchpad/wire-up-filter-subs-bug-655567 into devel. It's already in db-devel.

[Abel Deuring (adeuring)]

(18:18:58) allenap: adeuring: It's already reviewed and landed in db-devel, so I just need a +1 to land on devel.
(18:20:00) adeuring: allenap: so, rs=me
(18:20:10) allenap: adeuring: Thanks.

review: Approve (code)

=== modified file 'database/schema/security.cfg'
--- database/schema/security.cfg	2010-10-07 22:46:08 +0000
+++ database/schema/security.cfg	2010-10-15 16:17:07 +0000
@@ -549,6 +549,7 @@
 public.bugsubscriptionfilterstatus      = SELECT
 public.bugsubscriptionfilterimportance  = SELECT
 public.bugsubscriptionfiltertag         = SELECT
+public.bugtag                           = SELECT
 public.bugtask                          = SELECT, INSERT, UPDATE
 public.bugtracker                       = SELECT, INSERT
 public.bugtrackercomponent              = SELECT, INSERT, UPDATE, DELETE
@@ -638,6 +639,7 @@
 public.bugsubscriptionfiltertag         = SELECT
 public.bugnotification                  = SELECT, INSERT
 public.bugnotificationrecipient         = SELECT, INSERT
+public.bugtag                           = SELECT
 public.structuralsubscription           = SELECT
 public.message                          = SELECT, INSERT
 public.messagechunk                     = SELECT, INSERT
@@ -833,6 +835,7 @@
 public.bugnotification                  = SELECT, INSERT
 public.bugnotificationrecipient         = SELECT, INSERT
 public.bugnomination                    = SELECT
+public.bugtag                           = SELECT
 public.bugtask                          = SELECT, UPDATE
 public.product                          = SELECT
 public.project                          = SELECT
@@ -1194,6 +1197,7 @@
 public.bugnotification                  = SELECT, INSERT
 public.bugnotificationrecipient         = SELECT, INSERT
 public.bugnomination                    = SELECT
+public.bugtag                           = SELECT
 public.bugtask                          = SELECT, UPDATE
 public.product                          = SELECT, UPDATE
 public.project                          = SELECT, UPDATE
@@ -1300,6 +1304,7 @@
 public.bugnotification                  = SELECT, INSERT
 public.bugnotificationrecipient         = SELECT, INSERT
 public.bugnomination                    = SELECT
+public.bugtag                           = SELECT
 public.bugtask                          = SELECT, UPDATE
 public.product                          = SELECT, UPDATE
 public.project                          = SELECT, UPDATE
@@ -1855,6 +1860,7 @@
 public.bug                              = SELECT, INSERT, UPDATE
 public.bugjob                           = SELECT, INSERT
 public.bugaffectsperson                 = SELECT, INSERT, UPDATE, DELETE
+public.bugtag                           = SELECT
 public.bugtask                          = SELECT, INSERT, UPDATE
 public.accountpassword                  = SELECT, INSERT
 public.teamparticipation                = SELECT, INSERT
@@ -1917,6 +1923,7 @@
 public.bugaffectsperson                 = SELECT
 public.bugnotification                  = SELECT, DELETE
 public.bugnotificationrecipientarchive  = SELECT
+public.bugtag                           = SELECT
 public.bugwatch                         = SELECT, UPDATE
 public.bugwatchactivity                 = SELECT, DELETE
 public.codeimportresult                 = SELECT, DELETE

=== modified file 'lib/lp/bugs/configure.zcml'
--- lib/lp/bugs/configure.zcml	2010-10-06 18:53:53 +0000
+++ lib/lp/bugs/configure.zcml	2010-10-15 16:17:07 +0000
@@ -706,6 +706,7 @@
                     getSubscribersForPerson
                     indexed_messages
                     getAlsoNotifiedSubscribers
+                    getStructuralSubscribers
                     getBugWatch
                     canBeNominatedFor
                     getNominationFor

=== modified file 'lib/lp/bugs/interfaces/bug.py'
--- lib/lp/bugs/interfaces/bug.py	2010-10-12 14:56:31 +0000
+++ lib/lp/bugs/interfaces/bug.py	2010-10-15 16:17:07 +0000
@@ -493,6 +493,12 @@
         from duplicates.
         """
 
+    def getStructuralSubscribers(recipients=None, level=None):
+        """Return `IPerson`s subscribed to this bug's targets.
+
+        This takes into account bug subscription filters.
+        """
+
     def getSubscriptionsFromDuplicates():
         """Return IBugSubscriptions subscribed from dupes of this bug."""
 
@@ -501,9 +507,9 @@
 
     def getSubscribersForPerson(person):
         """Find the persons or teams by which person is subscribed.
-        
+
         This call should be quite cheap to make and performs a single query.
-        
+
         :return: An IResultSet.
         """
 

=== modified file 'lib/lp/bugs/model/bug.py'
--- lib/lp/bugs/model/bug.py	2010-10-14 11:05:05 +0000
+++ lib/lp/bugs/model/bug.py	2010-10-15 16:17:07 +0000
@@ -51,7 +51,6 @@
     In,
     LeftJoin,
     Max,
-    Min,
     Not,
     Or,
     Select,
@@ -156,14 +155,13 @@
 from lp.bugs.model.bugnomination import BugNomination
 from lp.bugs.model.bugnotification import BugNotification
 from lp.bugs.model.bugsubscription import BugSubscription
+from lp.bugs.model.bugtarget import OfficialBugTag
 from lp.bugs.model.bugtask import (
     BugTask,
     bugtask_sort_key,
-    BugTaskSet,
     get_bug_privacy_filter,
     NullBugTask,
     )
-from lp.bugs.model.bugtarget import OfficialBugTag
 from lp.bugs.model.bugwatch import BugWatch
 from lp.hardwaredb.interfaces.hwdb import IHWSubmissionBugSet
 from lp.registry.enum import BugNotificationLevel
@@ -172,10 +170,7 @@
     IDistributionSourcePackage,
     )
 from lp.registry.interfaces.distroseries import IDistroSeries
-from lp.registry.interfaces.person import (
-    IPersonSet,
-    validate_public_person,
-    )
+from lp.registry.interfaces.person import validate_public_person
 from lp.registry.interfaces.product import IProduct
 from lp.registry.interfaces.productseries import IProductSeries
 from lp.registry.interfaces.series import SeriesStatus
@@ -458,11 +453,9 @@
         store = Store.of(self)
         message_by_id = {}
         if include_parents:
-            def to_messages(rows):
-                return [row[0] for row in rows]
+            to_messages = lambda rows: [row[0] for row in rows]
         else:
-            def to_messages(rows):
-                return rows
+            to_messages = lambda rows: rows
         def eager_load_owners(messages):
             # Because we may have multiple owners, we spend less time in storm
             # with very large bugs by not joining and instead querying a second
@@ -976,31 +969,12 @@
 
         also_notified_subscribers = set()
 
-        structural_subscription_targets = set()
-
         for bugtask in self.bugtasks:
             if bugtask.assignee:
                 also_notified_subscribers.add(bugtask.assignee)
                 if recipients is not None:
                     recipients.addAssignee(bugtask.assignee)
 
-            if IStructuralSubscriptionTarget.providedBy(bugtask.target):
-                structural_subscription_targets.add(bugtask.target)
-                if bugtask.target.parent_subscription_target is not None:
-                    structural_subscription_targets.add(
-                        bugtask.target.parent_subscription_target)
-
-            if ISourcePackage.providedBy(bugtask.target):
-                # Distribution series bug tasks with a package have the
-                # source package set as their target, so we add the
-                # distroseries explicitly to the set of subscription
-                # targets.
-                structural_subscription_targets.add(
-                    bugtask.distroseries)
-
-            if bugtask.milestone is not None:
-                structural_subscription_targets.add(bugtask.milestone)
-
             # If the target's bug supervisor isn't set,
             # we add the owner as a subscriber.
             pillar = bugtask.pillar
@@ -1009,22 +983,73 @@
                 if recipients is not None:
                     recipients.addRegistrant(pillar.owner, pillar)
 
-        person_set = getUtility(IPersonSet)
-        target_subscribers = person_set.getSubscribersForTargets(
-            structural_subscription_targets, recipients=recipients,
-            level=level)
-
-        also_notified_subscribers.update(target_subscribers)
+        # Structural subscribers.
+        also_notified_subscribers.update(
+            self.getStructuralSubscribers(
+                recipients=recipients, level=level))
 
         # Direct subscriptions always take precedence over indirect
         # subscriptions.
         direct_subscribers = set(self.getDirectSubscribers())
+
         # Remove security proxy for the sort key, but return
         # the regular proxied object.
         return sorted(
             (also_notified_subscribers - direct_subscribers),
             key=lambda x: removeSecurityProxy(x).displayname)
 
+    def getStructuralSubscribers(self, recipients=None, level=None):
+        """See `IBug`. """
+        query_arguments = []
+        for bugtask in self.bugtasks:
+            if IStructuralSubscriptionTarget.providedBy(bugtask.target):
+                query_arguments.append((bugtask.target, bugtask))
+                if bugtask.target.parent_subscription_target is not None:
+                    query_arguments.append(
+                        (bugtask.target.parent_subscription_target, bugtask))
+            if ISourcePackage.providedBy(bugtask.target):
+                # Distribution series bug tasks with a package have the source
+                # package set as their target, so we add the distroseries
+                # explicitly to the set of subscription targets.
+                query_arguments.append((bugtask.distroseries, bugtask))
+            if bugtask.milestone is not None:
+                query_arguments.append((bugtask.milestone, bugtask))
+
+        if len(query_arguments) == 0:
+            return EmptyResultSet()
+
+        if level is None:
+            # If level is not specified, default to NOTHING so that all
+            # subscriptions are found. XXX: Perhaps this should go in
+            # getSubscriptionsForBugTask()?
+            level = BugNotificationLevel.NOTHING
+
+        # Build the query.
+        union = lambda left, right: left.union(right)
+        queries = (
+            target.getSubscriptionsForBugTask(bugtask, level)
+            for target, bugtask in query_arguments)
+        subscriptions = reduce(union, queries)
+
+        # Pull all the subscriptions in.
+        subscriptions = list(subscriptions)
+
+        # Prepare a query for the subscribers.
+        subscribers = Store.of(self).find(
+            Person, Person.id.is_in(
+                subscription.subscriberID
+                for subscription in subscriptions))
+
+        if recipients is not None:
+            # We need to process subscriptions, so pull all the subscribes
+            # into the cache, then update recipients with the subscriptions.
+            subscribers = list(subscribers)
+            for subscription in subscriptions:
+                recipients.addStructuralSubscriber(
+                    subscription.subscriber, subscription.target)
+
+        return subscribers
+
     def getBugNotificationRecipients(self, duplicateof=None, old_bug=None,
                                      level=None,
                                      include_master_dupe_subscribers=False):

=== renamed file 'lib/lp/bugs/tests/test_bug.py' => 'lib/lp/bugs/model/tests/test_bug.py'
--- lib/lp/bugs/tests/test_bug.py	2010-10-14 13:47:47 +0000
+++ lib/lp/bugs/model/tests/test_bug.py	2010-10-15 16:17:07 +0000
@@ -5,7 +5,10 @@
 
 __metaclass__ = type
 
+from storm.store import ResultSet
+
 from canonical.testing.layers import DatabaseFunctionalLayer
+from lp.bugs.mail.bugnotificationrecipients import BugNotificationRecipients
 from lp.registry.enum import BugNotificationLevel
 from lp.registry.interfaces.person import PersonVisibility
 from lp.registry.model.structuralsubscription import StructuralSubscription
@@ -14,6 +17,7 @@
     person_logged_in,
     TestCaseWithFactory,
     )
+from lp.testing.matchers import StartsWith
 
 
 class TestBug(TestCaseWithFactory):
@@ -36,7 +40,7 @@
         bug = self.factory.makeBug()
         person = self.factory.makePerson()
         team1 = self.factory.makeTeam(members=[person])
-        team2 = self.factory.makeTeam(members=[person])
+        self.factory.makeTeam(members=[person])
         with person_logged_in(person):
             bug.subscribe(team1, person)
         self.assertEqual([team1], list(bug.getSubscribersForPerson(person)))
@@ -157,8 +161,7 @@
             subscriber = self.factory.makePerson()
             subscribers.append(subscriber)
             with person_logged_in(subscriber):
-                subscription = bug.subscribe(
-                    subscriber, subscriber, level=level)
+                bug.subscribe(subscriber, subscriber, level=level)
             direct_subscribers = bug.getDirectSubscribers(level=level)
 
             # All the previous subscribers will be included because
@@ -181,8 +184,7 @@
             subscriber = self.factory.makePerson()
             subscribers.append(subscriber)
             with person_logged_in(subscriber):
-                subscription = bug.subscribe(
-                    subscriber, subscriber, level=level)
+                bug.subscribe(subscriber, subscriber, level=level)
 
         # All the subscribers should be returned by
         # getDirectSubscribers() because it defaults to returning
@@ -211,8 +213,7 @@
             subscriber = self.factory.makePerson()
             subscribers.append(subscriber)
             with person_logged_in(subscriber):
-                subscription = duplicate_bug.subscribe(
-                    subscriber, subscriber, level=level)
+                duplicate_bug.subscribe(subscriber, subscriber, level=level)
             duplicate_subscribers = (
                 bug.getSubscribersFromDuplicates(level=level))
             # All the previous subscribers will be included because
@@ -237,11 +238,92 @@
                 duplicate_bug.owner, duplicate_bug.owner)
         subscriber = self.factory.makePerson()
         with person_logged_in(subscriber):
-            direct_subscription = bug.subscribe(
+            bug.subscribe(
                 subscriber, subscriber, level=BugNotificationLevel.NOTHING)
-            dupe_subscription = duplicate_bug.subscribe(
+            duplicate_bug.subscribe(
                 subscriber, subscriber, level=BugNotificationLevel.METADATA)
         duplicate_subscribers = bug.getSubscribersFromDuplicates()
         self.assertTrue(
             subscriber not in duplicate_subscribers,
             "Subscriber should not be in duplicate_subscribers.")
+
+
+class TestBugStructuralSubscribers(TestCaseWithFactory):
+
+    layer = DatabaseFunctionalLayer
+
+    def test_getStructuralSubscribers_no_subscribers(self):
+        # If there are no subscribers for any of the bug's targets then no
+        # subscribers will be returned by getStructuralSubscribers().
+        product = self.factory.makeProduct()
+        bug = self.factory.makeBug(product=product)
+        subscribers = bug.getStructuralSubscribers()
+        self.assertIsInstance(subscribers, ResultSet)
+        self.assertEqual([], list(subscribers))
+
+    def test_getStructuralSubscribers_single_target(self):
+        # Subscribers for any of the bug's targets are returned.
+        subscriber = self.factory.makePerson()
+        login_person(subscriber)
+        product = self.factory.makeProduct()
+        product.addBugSubscription(subscriber, subscriber)
+        bug = self.factory.makeBug(product=product)
+        self.assertEqual([subscriber], list(bug.getStructuralSubscribers()))
+
+    def test_getStructuralSubscribers_multiple_targets(self):
+        # Subscribers for any of the bug's targets are returned.
+        actor = self.factory.makePerson()
+        login_person(actor)
+
+        subscriber1 = self.factory.makePerson()
+        subscriber2 = self.factory.makePerson()
+
+        product1 = self.factory.makeProduct(owner=actor)
+        product1.addBugSubscription(subscriber1, subscriber1)
+        product2 = self.factory.makeProduct(owner=actor)
+        product2.addBugSubscription(subscriber2, subscriber2)
+
+        bug = self.factory.makeBug(product=product1)
+        bug.addTask(actor, product2)
+
+        subscribers = bug.getStructuralSubscribers()
+        self.assertIsInstance(subscribers, ResultSet)
+        self.assertEqual(set([subscriber1, subscriber2]), set(subscribers))
+
+    def test_getStructuralSubscribers_recipients(self):
+        # If provided, getStructuralSubscribers() calls the appropriate
+        # methods on a BugNotificationRecipients object.
+        subscriber = self.factory.makePerson()
+        login_person(subscriber)
+        product = self.factory.makeProduct()
+        product.addBugSubscription(subscriber, subscriber)
+        bug = self.factory.makeBug(product=product)
+        recipients = BugNotificationRecipients()
+        subscribers = bug.getStructuralSubscribers(recipients=recipients)
+        # The return value is a list only when populating recipients.
+        self.assertIsInstance(subscribers, list)
+        self.assertEqual([subscriber], recipients.getRecipients())
+        reason, header = recipients.getReason(subscriber)
+        self.assertThat(
+            reason, StartsWith(
+                u"You received this bug notification because "
+                u"you are subscribed to "))
+        self.assertThat(header, StartsWith(u"Subscriber "))
+
+    def test_getStructuralSubscribers_level(self):
+        # getStructuralSubscribers() respects the given level.
+        subscriber = self.factory.makePerson()
+        login_person(subscriber)
+        product = self.factory.makeProduct()
+        subscription = product.addBugSubscription(subscriber, subscriber)
+        subscription.bug_notification_level = BugNotificationLevel.METADATA
+        bug = self.factory.makeBug(product=product)
+        self.assertEqual(
+            [subscriber], list(
+                bug.getStructuralSubscribers(
+                    level=BugNotificationLevel.METADATA)))
+        subscription.bug_notification_level = BugNotificationLevel.METADATA
+        self.assertEqual(
+            [], list(
+                bug.getStructuralSubscribers(
+                    level=BugNotificationLevel.COMMENTS)))

=== modified file 'lib/lp/registry/doc/structural-subscriptions.txt'
--- lib/lp/registry/doc/structural-subscriptions.txt	2010-10-06 18:53:53 +0000
+++ lib/lp/registry/doc/structural-subscriptions.txt	2010-10-15 16:17:07 +0000
@@ -198,61 +198,6 @@
     name16
 
 
-Retrieving structural subscribers of targets
-============================================
-
-Subscribers of one or more targets are retrieved by
-PersonSet.getSubscribersForTargets.
-
-XXX Abel Deuring 2008-07-11 We must remove the security proxy, because
-PersonSet.getSubscribersForTargets() accesses the private attribute
-firefox._targets_args. This attribute should be renamed. Bug #247525.
-
-    >>> from zope.security.proxy import removeSecurityProxy
-    >>> firefox_no_proxy = removeSecurityProxy(firefox)
-    >>> from lp.registry.interfaces.person import IPersonSet
-    >>> person_set = getUtility(IPersonSet)
-    >>> firefox_subscribers = person_set.getSubscribersForTargets(
-    ...     [firefox_no_proxy])
-    >>> print_bug_subscribers(firefox_subscribers)
-    name12
-
-If PersonSet.getSubscribersForTargets() is passed a
-BugNotificationLevel in its `level` parameter, only structural
-subscribers with that notification level or higher will be returned.
-The subscription level of ff_sub, the only structural subscription
-to firefox, is NOTHING, hence we get an empty list if we pass any
-larger bug notification level.
-
-    >>> firefox_subscribers = person_set.getSubscribersForTargets(
-    ...     [firefox_no_proxy], level=BugNotificationLevel.LIFECYCLE)
-    >>> print len(firefox_subscribers)
-    0
-
-If the bug notification level of ff_sub is increased, the subscription
-is included in the result of PersonSet.getSubscribersForTarget().
-
-    >>> ff_sub.level = BugNotificationLevel.LIFECYCLE
-    >>> firefox_subscribers = person_set.getSubscribersForTargets(
-    ...     [firefox_no_proxy])
-    >>> print_bug_subscribers(firefox_subscribers)
-    name12
-    >>> ff_sub.level = BugNotificationLevel.METADATA
-    >>> firefox_subscribers = person_set.getSubscribersForTargets(
-    ...     [firefox_no_proxy])
-    >>> print_bug_subscribers(firefox_subscribers)
-    name12
-
-More than one target can be passed to PersonSet.getSubscribersForTargets().
-
-    >>> ubuntu_no_proxy = removeSecurityProxy(ubuntu)
-    >>> ubuntu_or_firefox_subscribers = person_set.getSubscribersForTargets(
-    ...     [firefox_no_proxy, ubuntu_no_proxy])
-    >>> print_bug_subscribers(ubuntu_or_firefox_subscribers)
-    name12
-    name16
-
-
 Target type display
 ===================
 

=== modified file 'lib/lp/registry/interfaces/person.py'
--- lib/lp/registry/interfaces/person.py	2010-09-29 20:05:17 +0000
+++ lib/lp/registry/interfaces/person.py	2010-10-15 16:17:07 +0000
@@ -2006,17 +2006,6 @@
             specified product are returned.
         """
 
-    def getSubscribersForTargets(targets, recipients=None):
-        """Return the set of subscribers for `targets`.
-
-        :param targets: The sequence of targets for which to get the
-                        subscribers.
-        :param recipients: An optional instance of
-                           `BugNotificationRecipients`.
-                           If present, all found subscribers will be
-                           added to it.
-        """
-
     def updatePersonalStandings():
         """Update the personal standings of some people.
 

=== modified file 'lib/lp/registry/interfaces/structuralsubscription.py'
--- lib/lp/registry/interfaces/structuralsubscription.py	2010-10-05 09:23:22 +0000
+++ lib/lp/registry/interfaces/structuralsubscription.py	2010-10-15 16:17:07 +0000
@@ -200,8 +200,8 @@
     def userHasBugSubscriptions(user):
         """Is `user` subscribed, directly or via a team, to bug mail?"""
 
-    def getSubscriptionsForBug(bug, level):
-        """Return subscriptions for a given `IBug` at `level`."""
+    def getSubscriptionsForBugTask(bug, level):
+        """Return subscriptions for a given `IBugTask` at `level`."""
 
 
 class IStructuralSubscriptionTargetWrite(Interface):

=== modified file 'lib/lp/registry/model/person.py'
--- lib/lp/registry/model/person.py	2010-09-29 14:38:30 +0000
+++ lib/lp/registry/model/person.py	2010-10-15 16:17:07 +0000
@@ -4060,59 +4060,6 @@
             Person.id in (%s)
             ''' % branch_clause)
 
-    def getSubscribersForTargets(self, targets, recipients=None, level=None):
-        """See `IPersonSet`. """
-        if len(targets) == 0:
-            return set()
-        target_criteria = []
-        for target in targets:
-            # target_args is a mapping from query arguments
-            # to query values.
-            target_args = target._target_args
-            target_criteria_clauses = []
-            for key, value in target_args.items():
-                if value is not None:
-                    target_criteria_clauses.append(
-                        '%s = %s' % (key, quote(value)))
-                else:
-                    target_criteria_clauses.append(
-                        '%s IS NULL' % key)
-            if level is not None:
-                target_criteria_clauses.append('bug_notification_level >= %s'
-                    % quote(level.value))
-
-            target_criteria.append(
-                '(%s)' % ' AND '.join(target_criteria_clauses))
-
-        # Build a UNION query, since using OR slows down the query a lot.
-        subscriptions = StructuralSubscription.select(target_criteria[0])
-        for target_criterion in target_criteria[1:]:
-            subscriptions = subscriptions.union(
-                StructuralSubscription.select(target_criterion))
-
-        # Listify the result, since we want to loop over it multiple times.
-        subscriptions = list(subscriptions)
-
-        # We can't use prejoins in UNION queries, so populate the cache
-        # by getting all the subscribers.
-        subscriber_ids = [
-            subscription.subscriberID for subscription in subscriptions]
-        if len(subscriber_ids) > 0:
-            # Pull in ValidPersonCache records in addition to Person
-            # records to warm up the cache.
-            list(IStore(Person).find(
-                    (Person, ValidPersonCache),
-                    In(Person.id, subscriber_ids),
-                    ValidPersonCache.id == Person.id))
-
-        subscribers = set()
-        for subscription in subscriptions:
-            subscribers.add(subscription.subscriber)
-            if recipients is not None:
-                recipients.addStructuralSubscriber(
-                    subscription.subscriber, subscription.target)
-        return subscribers
-
     def updatePersonalStandings(self):
         """See `IPersonSet`."""
         cur = cursor()

=== modified file 'lib/lp/registry/model/structuralsubscription.py'
--- lib/lp/registry/model/structuralsubscription.py	2010-10-05 16:52:02 +0000
+++ lib/lp/registry/model/structuralsubscription.py	2010-10-15 16:17:07 +0000
@@ -484,21 +484,8 @@
                     return True
         return False
 
-    def getSubscriptionsForBug(self, bug, level):
+    def getSubscriptionsForBugTask(self, bugtask, level):
         """See `IStructuralSubscriptionTarget`."""
-        if IProjectGroup.providedBy(self):
-            targets = set(self.products)
-        elif IMilestone.providedBy(self):
-            targets = [self.series_target]
-        else:
-            targets = [self]
-
-        bugtasks = [
-            bugtask for bugtask in bug.bugtasks
-            if bugtask.target in targets]
-
-        assert len(bugtasks) != 0, repr(self)
-
         origin = [
             StructuralSubscription,
             LeftJoin(
@@ -515,7 +502,7 @@
                     BugSubscriptionFilter.id)),
             ]
 
-        if len(bug.tags) == 0:
+        if len(bugtask.bug.tags) == 0:
             tag_conditions = [
                 BugSubscriptionFilter.include_any_tags == False,
                 ]
@@ -534,13 +521,12 @@
                     # There's no status filter, or there is a status filter
                     # and and it matches.
                     Or(BugSubscriptionFilterStatus.id == None,
-                       BugSubscriptionFilterStatus.status.is_in(
-                            bugtask.status for bugtask in bugtasks)),
+                       BugSubscriptionFilterStatus.status == bugtask.status),
                     # There's no importance filter, or there is an importance
                     # filter and it matches.
                     Or(BugSubscriptionFilterImportance.id == None,
-                       BugSubscriptionFilterImportance.importance.is_in(
-                            bugtask.importance for bugtask in bugtasks)),
+                       BugSubscriptionFilterImportance.importance == (
+                            bugtask.importance)),
                     # Any number of conditions relating to tags.
                     *tag_conditions)),
             ]

=== modified file 'lib/lp/registry/tests/test_structuralsubscriptiontarget.py'
--- lib/lp/registry/tests/test_structuralsubscriptiontarget.py	2010-10-06 18:53:53 +0000
+++ lib/lp/registry/tests/test_structuralsubscriptiontarget.py	2010-10-15 16:17:07 +0000
@@ -173,19 +173,20 @@
     def makeBugTask(self):
         return self.factory.makeBugTask(target=self.target)
 
-    def test_getSubscriptionsForBug(self):
+    def test_getSubscriptionsForBugTask(self):
         # If no one has a filtered subscription for the given bug, the result
-        # of getSubscriptionsForBug() is the same as for getSubscriptions().
+        # of getSubscriptionsForBugTask() is the same as for
+        # getSubscriptions().
         bugtask = self.makeBugTask()
         subscriptions = self.target.getSubscriptions(
             min_bug_notification_level=BugNotificationLevel.NOTHING)
-        subscriptions_for_bug = self.target.getSubscriptionsForBug(
-            bugtask.bug, BugNotificationLevel.NOTHING)
-        self.assertEqual(list(subscriptions), list(subscriptions_for_bug))
+        subscriptions_for_bugtask = self.target.getSubscriptionsForBugTask(
+            bugtask, BugNotificationLevel.NOTHING)
+        self.assertEqual(list(subscriptions), list(subscriptions_for_bugtask))
 
-    def test_getSubscriptionsForBug_with_filter_on_status(self):
+    def test_getSubscriptionsForBugTask_with_filter_on_status(self):
         # If a status filter exists for a subscription, the result of
-        # getSubscriptionsForBug() may be a subset of getSubscriptions().
+        # getSubscriptionsForBugTask() may be a subset of getSubscriptions().
         bugtask = self.makeBugTask()
 
         # Create a new subscription on self.target.
@@ -195,9 +196,9 @@
         subscription.bug_notification_level = BugNotificationLevel.COMMENTS
 
         # Without any filters the subscription is found.
-        subscriptions_for_bug = self.target.getSubscriptionsForBug(
-            bugtask.bug, BugNotificationLevel.NOTHING)
-        self.assertEqual([subscription], list(subscriptions_for_bug))
+        subscriptions_for_bugtask = self.target.getSubscriptionsForBugTask(
+            bugtask, BugNotificationLevel.NOTHING)
+        self.assertEqual([subscription], list(subscriptions_for_bugtask))
 
         # Filter the subscription to bugs in the CONFIRMED state.
         subscription_filter = BugSubscriptionFilter()
@@ -205,19 +206,19 @@
         subscription_filter.statuses = [BugTaskStatus.CONFIRMED]
 
         # With the filter the subscription is not found.
-        subscriptions_for_bug = self.target.getSubscriptionsForBug(
-            bugtask.bug, BugNotificationLevel.NOTHING)
-        self.assertEqual([], list(subscriptions_for_bug))
+        subscriptions_for_bugtask = self.target.getSubscriptionsForBugTask(
+            bugtask, BugNotificationLevel.NOTHING)
+        self.assertEqual([], list(subscriptions_for_bugtask))
 
         # If the filter is adjusted, the subscription is found again.
         subscription_filter.statuses = [bugtask.status]
-        subscriptions_for_bug = self.target.getSubscriptionsForBug(
-            bugtask.bug, BugNotificationLevel.NOTHING)
-        self.assertEqual([subscription], list(subscriptions_for_bug))
+        subscriptions_for_bugtask = self.target.getSubscriptionsForBugTask(
+            bugtask, BugNotificationLevel.NOTHING)
+        self.assertEqual([subscription], list(subscriptions_for_bugtask))
 
-    def test_getSubscriptionsForBug_with_filter_on_importance(self):
+    def test_getSubscriptionsForBugTask_with_filter_on_importance(self):
         # If an importance filter exists for a subscription, the result of
-        # getSubscriptionsForBug() may be a subset of getSubscriptions().
+        # getSubscriptionsForBugTask() may be a subset of getSubscriptions().
         bugtask = self.makeBugTask()
 
         # Create a new subscription on self.target.
@@ -227,9 +228,9 @@
         subscription.bug_notification_level = BugNotificationLevel.COMMENTS
 
         # Without any filters the subscription is found.
-        subscriptions_for_bug = self.target.getSubscriptionsForBug(
-            bugtask.bug, BugNotificationLevel.NOTHING)
-        self.assertEqual([subscription], list(subscriptions_for_bug))
+        subscriptions_for_bugtask = self.target.getSubscriptionsForBugTask(
+            bugtask, BugNotificationLevel.NOTHING)
+        self.assertEqual([subscription], list(subscriptions_for_bugtask))
 
         # Filter the subscription to bugs in the CRITICAL state.
         subscription_filter = BugSubscriptionFilter()
@@ -237,19 +238,19 @@
         subscription_filter.importances = [BugTaskImportance.CRITICAL]
 
         # With the filter the subscription is not found.
-        subscriptions_for_bug = self.target.getSubscriptionsForBug(
-            bugtask.bug, BugNotificationLevel.NOTHING)
-        self.assertEqual([], list(subscriptions_for_bug))
+        subscriptions_for_bugtask = self.target.getSubscriptionsForBugTask(
+            bugtask, BugNotificationLevel.NOTHING)
+        self.assertEqual([], list(subscriptions_for_bugtask))
 
         # If the filter is adjusted, the subscription is found again.
         subscription_filter.importances = [bugtask.importance]
-        subscriptions_for_bug = self.target.getSubscriptionsForBug(
-            bugtask.bug, BugNotificationLevel.NOTHING)
-        self.assertEqual([subscription], list(subscriptions_for_bug))
+        subscriptions_for_bugtask = self.target.getSubscriptionsForBugTask(
+            bugtask, BugNotificationLevel.NOTHING)
+        self.assertEqual([subscription], list(subscriptions_for_bugtask))
 
-    def test_getSubscriptionsForBug_with_filter_on_level(self):
+    def test_getSubscriptionsForBugTask_with_filter_on_level(self):
         # All structural subscriptions have a level for bug notifications
-        # which getSubscriptionsForBug() observes.
+        # which getSubscriptionsForBugTask() observes.
         bugtask = self.makeBugTask()
 
         # Create a new METADATA level subscription on self.target.
@@ -259,19 +260,19 @@
         subscription.bug_notification_level = BugNotificationLevel.METADATA
 
         # The subscription is found when looking for NOTHING or above.
-        subscriptions_for_bug = self.target.getSubscriptionsForBug(
-            bugtask.bug, BugNotificationLevel.NOTHING)
-        self.assertEqual([subscription], list(subscriptions_for_bug))
+        subscriptions_for_bugtask = self.target.getSubscriptionsForBugTask(
+            bugtask, BugNotificationLevel.NOTHING)
+        self.assertEqual([subscription], list(subscriptions_for_bugtask))
         # The subscription is found when looking for METADATA or above.
-        subscriptions_for_bug = self.target.getSubscriptionsForBug(
-            bugtask.bug, BugNotificationLevel.METADATA)
-        self.assertEqual([subscription], list(subscriptions_for_bug))
+        subscriptions_for_bugtask = self.target.getSubscriptionsForBugTask(
+            bugtask, BugNotificationLevel.METADATA)
+        self.assertEqual([subscription], list(subscriptions_for_bugtask))
         # The subscription is not found when looking for COMMENTS or above.
-        subscriptions_for_bug = self.target.getSubscriptionsForBug(
-            bugtask.bug, BugNotificationLevel.COMMENTS)
-        self.assertEqual([], list(subscriptions_for_bug))
+        subscriptions_for_bugtask = self.target.getSubscriptionsForBugTask(
+            bugtask, BugNotificationLevel.COMMENTS)
+        self.assertEqual([], list(subscriptions_for_bugtask))
 
-    def test_getSubscriptionsForBug_with_filter_include_any_tags(self):
+    def test_getSubscriptionsForBugTask_with_filter_include_any_tags(self):
         # If a subscription filter has include_any_tags, a bug with one or
         # more tags is matched.
         bugtask = self.makeBugTask()
@@ -285,17 +286,17 @@
         subscription_filter.include_any_tags = True
 
         # Without any tags the subscription is not found.
-        subscriptions_for_bug = self.target.getSubscriptionsForBug(
-            bugtask.bug, BugNotificationLevel.NOTHING)
-        self.assertEqual([], list(subscriptions_for_bug))
+        subscriptions_for_bugtask = self.target.getSubscriptionsForBugTask(
+            bugtask, BugNotificationLevel.NOTHING)
+        self.assertEqual([], list(subscriptions_for_bugtask))
 
         # With any tag the subscription is found.
         bugtask.bug.tags = ["foo"]
-        subscriptions_for_bug = self.target.getSubscriptionsForBug(
-            bugtask.bug, BugNotificationLevel.NOTHING)
-        self.assertEqual([subscription], list(subscriptions_for_bug))
+        subscriptions_for_bugtask = self.target.getSubscriptionsForBugTask(
+            bugtask, BugNotificationLevel.NOTHING)
+        self.assertEqual([subscription], list(subscriptions_for_bugtask))
 
-    def test_getSubscriptionsForBug_with_filter_exclude_any_tags(self):
+    def test_getSubscriptionsForBugTask_with_filter_exclude_any_tags(self):
         # If a subscription filter has exclude_any_tags, only bugs with no
         # tags are matched.
         bugtask = self.makeBugTask()
@@ -309,17 +310,17 @@
         subscription_filter.exclude_any_tags = True
 
         # Without any tags the subscription is found.
-        subscriptions_for_bug = self.target.getSubscriptionsForBug(
-            bugtask.bug, BugNotificationLevel.NOTHING)
-        self.assertEqual([subscription], list(subscriptions_for_bug))
+        subscriptions_for_bugtask = self.target.getSubscriptionsForBugTask(
+            bugtask, BugNotificationLevel.NOTHING)
+        self.assertEqual([subscription], list(subscriptions_for_bugtask))
 
         # With any tag the subscription is not found.
         bugtask.bug.tags = ["foo"]
-        subscriptions_for_bug = self.target.getSubscriptionsForBug(
-            bugtask.bug, BugNotificationLevel.NOTHING)
-        self.assertEqual([], list(subscriptions_for_bug))
+        subscriptions_for_bugtask = self.target.getSubscriptionsForBugTask(
+            bugtask, BugNotificationLevel.NOTHING)
+        self.assertEqual([], list(subscriptions_for_bugtask))
 
-    def test_getSubscriptionsForBug_with_multiple_filters(self):
+    def test_getSubscriptionsForBugTask_with_multiple_filters(self):
         # If multiple filters exist for a subscription, all filters must
         # match.
         bugtask = self.makeBugTask()
@@ -337,23 +338,23 @@
         subscription_filter.importances = [BugTaskImportance.CRITICAL]
 
         # With the filter the subscription is not found.
-        subscriptions_for_bug = self.target.getSubscriptionsForBug(
-            bugtask.bug, BugNotificationLevel.NOTHING)
-        self.assertEqual([], list(subscriptions_for_bug))
+        subscriptions_for_bugtask = self.target.getSubscriptionsForBugTask(
+            bugtask, BugNotificationLevel.NOTHING)
+        self.assertEqual([], list(subscriptions_for_bugtask))
 
         # If the filter is adjusted to match status but not importance, the
         # subscription is still not found.
         subscription_filter.statuses = [bugtask.status]
-        subscriptions_for_bug = self.target.getSubscriptionsForBug(
-            bugtask.bug, BugNotificationLevel.NOTHING)
-        self.assertEqual([], list(subscriptions_for_bug))
+        subscriptions_for_bugtask = self.target.getSubscriptionsForBugTask(
+            bugtask, BugNotificationLevel.NOTHING)
+        self.assertEqual([], list(subscriptions_for_bugtask))
 
         # If the filter is adjusted to also match importance, the subscription
         # is found again.
         subscription_filter.importances = [bugtask.importance]
-        subscriptions_for_bug = self.target.getSubscriptionsForBug(
-            bugtask.bug, BugNotificationLevel.NOTHING)
-        self.assertEqual([subscription], list(subscriptions_for_bug))
+        subscriptions_for_bugtask = self.target.getSubscriptionsForBugTask(
+            bugtask, BugNotificationLevel.NOTHING)
+        self.assertEqual([subscription], list(subscriptions_for_bugtask))
 
 
 class TestStructuralSubscriptionForDistro(

=== modified file 'utilities/format-new-and-modified-imports'
--- utilities/format-new-and-modified-imports	2010-09-28 09:50:49 +0000
+++ utilities/format-new-and-modified-imports	2010-10-15 16:17:07 +0000
@@ -9,5 +9,5 @@
 #
 
 bzr status --short "$@" | \
-    awk '/^.[MN] .*[.]py$/ { print $2 }' | \
+    awk '/^.[MN] .*[.]py$/ { print $NF }' | \
     xargs -r "$(dirname "$0")/format-imports"