launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #03651
[Merge] lp:~gmb/launchpad/rollback-13050 into lp:launchpad
Graham Binns has proposed merging lp:~gmb/launchpad/rollback-13050 into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~gmb/launchpad/rollback-13050/+merge/61265
This branch reverts devel r13050, which may have resulted in bug 783948 manifesting.
--
https://code.launchpad.net/~gmb/launchpad/rollback-13050/+merge/61265
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~gmb/launchpad/rollback-13050 into lp:launchpad.
=== 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))
