Merge lp:~gmb/launchpad/rollback-13050 into lp:launchpad

[Graham Binns]

[r=gmb][ui=none][bug=783948] [rollback=13050] Revert bad revision r13050, which may have caused bug 783948.

This branch reverts devel r13050, which may have resulted in bug 783948 manifesting.

[Graham Binns (gmb)]

Self-approving after checking that rolling back fixed the problem.

review: Approve

=== modified file 'lib/lp/bugs/browser/bug.py'
--- lib/lp/bugs/browser/bug.py	2011-05-16 17:43:29 +0000
+++ lib/lp/bugs/browser/bug.py	2011-05-17 15:03:32 +0000
@@ -536,7 +536,6 @@
     @cachedproperty
     def user_should_see_mute_link(self):
         """Return True if the user should see the Mute link."""
-        user_is_subscribed = False
         if features.getFeatureFlag('malone.advanced-subscriptions.enabled'):
             user_is_subscribed = (
                 # Note that we don't have to check for isMuted(), since
@@ -545,7 +544,9 @@
                 self.context.isSubscribed(self.user) or
                 self.context.isSubscribedToDupes(self.user) or
                 self.context.personIsAlsoNotifiedSubscriber(self.user))
-        return user_is_subscribed
+            return user_is_subscribed
+        else:
+            return False
 
     @cachedproperty
     def _bug_attachments(self):

=== modified file 'lib/lp/bugs/browser/tests/test_bug_views.py'
--- lib/lp/bugs/browser/tests/test_bug_views.py	2011-05-12 10:36:43 +0000
+++ lib/lp/bugs/browser/tests/test_bug_views.py	2011-05-17 15:03:32 +0000
@@ -177,64 +177,3 @@
                 self.assertTrue(
                     self._hasCSSClass(html, 'mute-link-container', 'hidden'),
                     'No "hidden" CSS class in mute-link-container.')
-
-    def test_mute_link_shown_for_team_subscription_to_duplicate(self):
-        # If the person belongs to a team with a structural subscription to a
-        # duplicate of this bug, then the mute link will be displayed to them.
-        person = self.factory.makePerson(name="a-person")
-        team_owner = self.factory.makePerson(name="team-owner")
-        team = self.factory.makeTeam(owner=team_owner, name="subscribed-team")
-        dupe = self.factory.makeBug()
-        with person_logged_in(self.bug.owner):
-            dupe.markAsDuplicate(self.bug)
-        with FeatureFixture({self.feature_flag_1: 'on'}):
-            with person_logged_in(team_owner):
-                team.addMember(person, team_owner)
-                dupe_target = dupe.default_bugtask.target
-                dupe_target.addBugSubscription(team, team_owner)
-            with person_logged_in(person):
-                self.assertFalse(self.bug.isMuted(person))
-                # This is a sanity check for the test.
-                self.assertFalse(self.bug.duplicates.is_empty())
-                self.assertEqual(self.bug, dupe.duplicateof)
-                self.assertTrue(
-                    self.bug.personIsAlsoNotifiedSubscriber(
-                        person), "Person should be a notified subscriber")
-                view = create_initialized_view(
-                    self.bug, name="+portlet-subscribers")
-                self.assertTrue(view.user_should_see_mute_link,
-                                "User should see mute link.")
-                contents = view.render()
-                self.assertTrue('mute_subscription' in contents,
-                                "'mute_subscription' not in contents.")
-                create_initialized_view(
-                    self.bug.default_bugtask, name="+mute",
-                    form={'field.actions.mute': 'Mute bug mail'})
-                self.assertTrue(self.bug.isMuted(person))
-
-    def test_mute_subscription_link_shown_for_team_direct_subscription(self):
-        # If the person belongs to a team with a direct subscription,
-        # then the mute link will be displayed to them.
-        person = self.factory.makePerson(name="a-person")
-        team_owner = self.factory.makePerson(name="team-owner")
-        team = self.factory.makeTeam(owner=team_owner, name="subscribed-team")
-        with FeatureFixture({self.feature_flag_1: 'on'}):
-            with person_logged_in(team_owner):
-                team.addMember(person, team_owner)
-                self.bug.subscribe(team, team_owner)
-            with person_logged_in(person):
-                self.assertFalse(self.bug.isMuted(person))
-                self.assertTrue(
-                    self.bug.personIsAlsoNotifiedSubscriber(person),
-                    "Person should be a notified subscriber")
-                view = create_initialized_view(
-                    self.bug, name="+portlet-subscribers")
-                self.assertTrue(view.user_should_see_mute_link,
-                                "User should see mute link.")
-                contents = view.render()
-                self.assertTrue('mute_subscription' in contents,
-                                "'mute_subscription' not in contents.")
-                create_initialized_view(
-                    self.bug.default_bugtask, name="+mute",
-                    form={'field.actions.mute': 'Mute bug mail'})
-                self.assertTrue(self.bug.isMuted(person))

=== modified file 'lib/lp/bugs/doc/bugsubscription.txt'
--- lib/lp/bugs/doc/bugsubscription.txt	2011-05-16 17:40:00 +0000
+++ lib/lp/bugs/doc/bugsubscription.txt	2011-05-17 15:03:32 +0000
@@ -423,7 +423,7 @@
     []
 
     >>> linux_source_bug.getSubscribersFromDuplicates()
-    []
+    ()
 
 Direct subscriptions always take precedence over indirect
 subscriptions. So, if we unmark the above bug as private,

=== modified file 'lib/lp/bugs/model/bug.py'
--- lib/lp/bugs/model/bug.py	2011-05-17 05:43:19 +0000
+++ lib/lp/bugs/model/bug.py	2011-05-17 15:03:32 +0000
@@ -953,30 +953,18 @@
         See the comment in getDirectSubscribers for a description of the
         recipients argument.
         """
-        if self.private:
-            # We short-circuit for private bugs, since actually
-            # returning something non-empty here causes things to break
-            # in fun and interesting ways (see bug 780248).
-            return []
-
         if level is None:
             level = BugNotificationLevel.NOTHING
         info = self.getSubscriptionInfo(level)
 
         if recipients is not None:
-            # Pre-load duplicates
+            # Pre-load duplicate bugs.
             list(self.duplicates)
             for subscription in info.duplicate_only_subscriptions:
                 recipients.addDupeSubscriber(
                     subscription.person, subscription.bug)
-            for subscription in info.structural_subscriptions_from_duplicates:
-                recipients.addDupeSubscriber(
-                    subscription.subscriber)
 
-        unified_subscribers = (
-            info.duplicate_only_subscriptions.subscribers.union(
-                info.structural_subscriptions_from_duplicates.subscribers))
-        return unified_subscribers.sorted
+        return info.duplicate_only_subscriptions.subscribers.sorted
 
     def getSubscribersForPerson(self, person):
         """See `IBug."""
@@ -1041,16 +1029,12 @@
                                      include_master_dupe_subscribers=False):
         """See `IBug`."""
         recipients = BugNotificationRecipients(duplicateof=duplicateof)
-        # Call getDirectSubscribers to update the recipients list with direct
-        # subscribers.  The results of the method call are not used.
         self.getDirectSubscribers(recipients, level=level)
         if self.private:
             assert self.getIndirectSubscribers() == [], (
                 "Indirect subscribers found on private bug. "
                 "A private bug should never have implicit subscribers!")
         else:
-            # Call getIndirectSubscribers to update the recipients list with direct
-            # subscribers.  The results of the method call are not used.
             self.getIndirectSubscribers(recipients, level=level)
             if include_master_dupe_subscribers and self.duplicateof:
                 # This bug is a public duplicate of another bug, so include
@@ -1917,22 +1901,6 @@
 
     def personIsAlsoNotifiedSubscriber(self, person):
         """See `IBug`."""
-        # This is here to avoid circular imports.
-        from lp.bugs.mail.bugnotificationrecipients import (
-            BugNotificationRecipients,
-            )
-
-        def check_person_in_team(person, list_of_people):
-            """Is the person in one of the teams?
-
-            Given a person and a list of people/teams, see if the person
-            belongs to one of the teams.
-            """
-            for subscriber in list_of_people:
-                if subscriber.is_team and person.inTeam(subscriber):
-                    return True
-            return False
-
         # We have to use getAlsoNotifiedSubscribers() here and iterate
         # over what it returns because "also notified subscribers" is
         # actually a composite of bug contacts, structural subscribers
@@ -1943,16 +1911,9 @@
             return True
         # Otherwise check to see if the person is a member of any of the
         # subscribed teams.
-        if check_person_in_team(person, also_notified_subscribers):
-            return True
-
-        direct_subscribers = self.getDirectSubscribers()
-        if check_person_in_team(person, direct_subscribers):
-            return True
-        duplicate_subscribers = self.getSubscribersFromDuplicates(
-            recipients=BugNotificationRecipients())
-        if check_person_in_team(person, duplicate_subscribers):
-            return True
+        for subscriber in also_notified_subscribers:
+            if subscriber.is_team and person.inTeam(subscriber):
+                return True
         return False
 
     def personIsSubscribedToDuplicate(self, person):
@@ -2312,24 +2273,6 @@
         return get_structural_subscriptions_for_bug(self.bug)
 
     @cachedproperty
-    @freeze(StructuralSubscriptionSet)
-    def structural_subscriptions_from_duplicates(self):
-        """Structural subscriptions from the bug's duplicates."""
-        self.duplicate_subscriptions.subscribers # Pre-load subscribers.
-        higher_precedence = (
-            self.direct_subscriptions.subscribers.union(
-                self.also_notified_subscribers))
-        all_duplicate_structural_subscriptions = list()
-        for duplicate in self.bug.duplicates:
-            duplicate_struct_subs = get_structural_subscriptions_for_bug(
-                duplicate)
-            all_duplicate_structural_subscriptions += duplicate_struct_subs
-        return (
-            subscription for subscription in
-                all_duplicate_structural_subscriptions
-                if subscription.subscriber not in higher_precedence)
-
-    @cachedproperty
     @freeze(BugSubscriberSet)
     def all_assignees(self):
         """Assignees of the bug's tasks."""

=== modified file 'lib/lp/bugs/tests/test_bugsubscription.py'
--- lib/lp/bugs/tests/test_bugsubscription.py	2011-05-10 14:13:24 +0000
+++ lib/lp/bugs/tests/test_bugsubscription.py	2011-05-17 15:03:32 +0000
@@ -146,26 +146,3 @@
                 self.bug.id, team_2.name, self.subscriber)
         self.assertThat(
             collector, HasQueryCount(LessThan(25)))
-
-
-class TestBugSubscriptionMethods(TestCaseWithFactory):
-    """A TestCase for the subscription-related methods of `Bug`."""
-
-    layer = DatabaseFunctionalLayer
-
-    def test_getSubscribersFromDuplicates_returns_empty_when_private(self):
-        # If a private bug has duplicates its
-        # getSubscribersFromDuplicates() method should return an empty
-        # set.
-        # This is a regression test for bug 780248.
-        bug = self.factory.makeBug()
-        duplicate = self.factory.makeBug()
-        team = self.factory.makeTeam()
-        with person_logged_in(team.teamowner):
-            duplicate.default_bugtask.product.addSubscription(
-                team, team.teamowner)
-        with person_logged_in(bug.owner):
-            bug.setPrivate(True, bug.owner)
-            duplicate.markAsDuplicate(bug)
-            duplicate_subscribers = bug.getSubscribersFromDuplicates()
-            self.assertEqual(0, len(duplicate_subscribers))