launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #06024
lp:~allenap/launchpad/bugnomination-timeout-bug-874250-preload-email into lp:launchpad
Gavin Panella has proposed merging lp:~allenap/launchpad/bugnomination-timeout-bug-874250-preload-email into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #874250 in Launchpad itself: "BugNomination:+editstatus timeout for bugs with many tasks"
https://bugs.launchpad.net/launchpad/+bug/874250
For more details, see:
https://code.launchpad.net/~allenap/launchpad/bugnomination-timeout-bug-874250-preload-email/+merge/87397
This branch is long, but is actually quite readable, I think :)
One big problem when updating bugs is sending notifications to
subscribers. It looks like preferred email addresses are being loaded
one by one (though I started this branch long enough ago that the
details have faded).
In getting email addresses preloaded I updated BugSubscriptionInfo,
fixed some issues with it, and gotten get_also_notified_subscribers()
using it.
I think BSI is now a fairly good and comprehensive foundation for any
bug subscription calculation. It may not do everything in the minimum
number of queries possible, but it's does everything in a constant
number of queries, and makes it easy for code that uses it to also do
so.
There are still methods in IBug (and probably elsewhere) that could be
refactored to use BSI more directly, but I'll leave that for another
time.
The changes:
- Updates BugSubscriptionInfo:
- Adds all_direct_subscriptions, all_direct_subscribers,
direct_subscribers, muted_subscribers, and structural_subscribers
properties.
- Adds support for returning subscription info for only a single
bugtask as well as all bugtasks of the given bug.
- Adds forLevel() and forTask() methods.
- When loading Person records, load preferred email information too.
- Updates get_also_notified_subscribers():
- Use BugSubscriptionInfo more heavily.
- Adds tests around performance of this function when passed a
recipients argument. This was previously poor (potato potato).
- Updates structural subscriptions:
Split get_structural_subscribers() into two functions -
get_structural_subscribers() and get_structural_subscriptions() -
which both back onto query_structural_subscriptions(), a new
function. This supports the changes to BugSubscriptionInfo and also
makes the implementation cleaner.
--
https://code.launchpad.net/~allenap/launchpad/bugnomination-timeout-bug-874250-preload-email/+merge/87397
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~allenap/launchpad/bugnomination-timeout-bug-874250-preload-email into lp:launchpad.
=== modified file 'lib/lp/bugs/configure.zcml'
--- lib/lp/bugs/configure.zcml 2011-12-30 08:03:42 +0000
+++ lib/lp/bugs/configure.zcml 2012-01-03 17:52:00 +0000
@@ -650,6 +650,7 @@
permission="launchpad.View"
attributes="
bug
+ bugtask
level
" />
<!-- Properties -->
@@ -657,13 +658,25 @@
permission="launchpad.View"
attributes="
all_assignees
+ all_direct_subscriptions
+ all_direct_subscribers
all_pillar_owners_without_bug_supervisors
also_notified_subscribers
direct_subscriptions
+ direct_subscribers
duplicate_only_subscriptions
duplicate_subscriptions
indirect_subscribers
+ muted_subscribers
structural_subscriptions
+ structural_subscribers
+ " />
+ <!-- Methods -->
+ <require
+ permission="launchpad.View"
+ attributes="
+ forLevel
+ forTask
" />
</class>
=== modified file 'lib/lp/bugs/doc/bugsubscription.txt'
--- lib/lp/bugs/doc/bugsubscription.txt 2011-12-24 17:49:30 +0000
+++ lib/lp/bugs/doc/bugsubscription.txt 2012-01-03 17:52:00 +0000
@@ -99,7 +99,7 @@
getAlsoNotifiedSubscribers() and getSubscribersFromDuplicates().
>>> linux_source_bug.getAlsoNotifiedSubscribers()
- [<Person at ...>]
+ (<Person at ...>,)
>>> linux_source_bug.getSubscribersFromDuplicates()
()
@@ -109,7 +109,7 @@
>>> from lp.bugs.model.bug import get_also_notified_subscribers
>>> res = get_also_notified_subscribers(linux_source_bug.bugtasks[0])
>>> res
- [<Person at ...>]
+ (<Person at ...>,)
These are security proxied.
=== modified file 'lib/lp/bugs/interfaces/bug.py'
--- lib/lp/bugs/interfaces/bug.py 2012-01-01 02:58:52 +0000
+++ lib/lp/bugs/interfaces/bug.py 2012-01-03 17:52:00 +0000
@@ -578,10 +578,11 @@
If no such `BugSubscription` exists, return None.
"""
- def getSubscriptionInfo(level):
+ def getSubscriptionInfo(level=None):
"""Return a `BugSubscriptionInfo` at the given `level`.
- :param level: A member of `BugNotificationLevel`.
+ :param level: A member of `BugNotificationLevel`. Defaults to
+ `BugSubscriptionLevel.LIFECYCLE` if unspecified.
"""
def getBugNotificationRecipients(duplicateof=None, old_bug=None,
=== modified file 'lib/lp/bugs/interfaces/bugtask.py'
--- lib/lp/bugs/interfaces/bugtask.py 2012-01-01 02:58:52 +0000
+++ lib/lp/bugs/interfaces/bugtask.py 2012-01-03 17:52:00 +0000
@@ -560,7 +560,7 @@
title=_('Assigned to'), required=False,
vocabulary='ValidAssignee',
readonly=True))
- assigneeID = Attribute('The assignee ID (for eager loading)')
+ assigneeID = Int(title=_('The assignee ID (for eager loading)'))
bugtargetdisplayname = exported(
Text(title=_("The short, descriptive name of the target"),
readonly=True),
=== modified file 'lib/lp/bugs/model/bug.py'
--- lib/lp/bugs/model/bug.py 2011-12-30 06:14:56 +0000
+++ lib/lp/bugs/model/bug.py 2012-01-03 17:52:00 +0000
@@ -158,6 +158,7 @@
from lp.bugs.model.bugwatch import BugWatch
from lp.bugs.model.structuralsubscription import (
get_structural_subscribers,
+ get_structural_subscriptions,
get_structural_subscriptions_for_bug,
)
from lp.code.interfaces.branchcollection import IAllBranches
@@ -948,9 +949,10 @@
BugSubscription.bug_id == self.id).order_by(BugSubscription.id)
return DecoratedResultSet(results, operator.itemgetter(1))
- def getSubscriptionInfo(self, level=BugNotificationLevel.LIFECYCLE):
+ def getSubscriptionInfo(self, level=None):
"""See `IBug`."""
- return BugSubscriptionInfo(self, level)
+ return BugSubscriptionInfo(
+ self, BugNotificationLevel.LIFECYCLE if level is None else level)
def getDirectSubscriptions(self):
"""See `IBug`."""
@@ -1002,6 +1004,8 @@
# the regular proxied object.
return sorted(
indirect_subscribers,
+ # XXX: GavinPanella 2011-12-12 bug=???: Should probably use
+ # person_sort_key.
key=lambda x: removeSecurityProxy(x).displayname)
def getSubscriptionsFromDuplicates(self, recipients=None):
@@ -1030,14 +1034,13 @@
if level is None:
level = BugNotificationLevel.LIFECYCLE
info = self.getSubscriptionInfo(level)
-
if recipients is not None:
- # Pre-load duplicate bugs.
- list(self.duplicates)
+ list(self.duplicates) # Pre-load duplicate bugs.
+ info.duplicate_only_subscribers # Pre-load subscribers.
for subscription in info.duplicate_only_subscriptions:
recipients.addDupeSubscriber(
subscription.person, subscription.bug)
- return info.duplicate_only_subscriptions.subscribers.sorted
+ return info.duplicate_only_subscribers.sorted
def getSubscribersForPerson(self, person):
"""See `IBug."""
@@ -2389,48 +2392,44 @@
if IBug.providedBy(bug_or_bugtask):
bug = bug_or_bugtask
bugtasks = bug.bugtasks
+ info = bug.getSubscriptionInfo(level)
elif IBugTask.providedBy(bug_or_bugtask):
bug = bug_or_bugtask.bug
bugtasks = [bug_or_bugtask]
+ info = bug.getSubscriptionInfo(level).forTask(bug_or_bugtask)
else:
raise ValueError('First argument must be bug or bugtask')
if bug.private:
return []
- # Direct subscriptions always take precedence over indirect
- # subscriptions.
- direct_subscribers = set(bug.getDirectSubscribers())
-
- also_notified_subscribers = set()
-
- for bugtask in bugtasks:
- if (bugtask.assignee and
- bugtask.assignee not in direct_subscribers):
- # We have an assignee that is not a direct subscriber.
- also_notified_subscribers.add(bugtask.assignee)
- if recipients is not None:
+ # Subscribers to exclude.
+ exclude_subscribers = frozenset().union(
+ info.all_direct_subscribers, info.muted_subscribers)
+ # Get also notified subscribers at the given level for the given tasks.
+ also_notified_subscribers = info.also_notified_subscribers
+
+ if recipients is not None:
+ for bugtask in bugtasks:
+ assignee = bugtask.assignee
+ if assignee in also_notified_subscribers:
+ # We have an assignee that is not a direct subscriber.
recipients.addAssignee(bugtask.assignee)
-
- # If the target's bug supervisor isn't set...
- pillar = bugtask.pillar
- if (pillar.bug_supervisor is None and
- pillar.official_malone and
- pillar.owner not in direct_subscribers):
- # ...we add the owner as a subscriber.
- also_notified_subscribers.add(pillar.owner)
- if recipients is not None:
- recipients.addRegistrant(pillar.owner, pillar)
+ # If the target's bug supervisor isn't set...
+ pillar = bugtask.pillar
+ if pillar.official_malone and pillar.bug_supervisor is None:
+ if pillar.owner in also_notified_subscribers:
+ # ...we add the owner as a subscriber.
+ recipients.addRegistrant(pillar.owner, pillar)
# This structural subscribers code omits direct subscribers itself.
- also_notified_subscribers.update(
- get_structural_subscribers(
- bug_or_bugtask, recipients, level, direct_subscribers))
+ # TODO: Pass the info object into get_structural_subscribers for
+ # efficiency... or do the recipients stuff here.
+ structural_subscribers = get_structural_subscribers(
+ bug_or_bugtask, recipients, level, exclude_subscribers)
+ assert also_notified_subscribers.issuperset(structural_subscribers)
- # Remove security proxy for the sort key, but return
- # the regular proxied object.
- return sorted(also_notified_subscribers,
- key=lambda x: removeSecurityProxy(x).displayname)
+ return also_notified_subscribers.sorted
def load_people(*where):
@@ -2443,7 +2442,8 @@
`ValidPersonCache` records are loaded simultaneously.
"""
return PersonSet()._getPrecachedPersons(
- origin=[Person], conditions=where, need_validity=True)
+ origin=[Person], conditions=where, need_validity=True,
+ need_preferred_email=True)
class BugSubscriberSet(frozenset):
@@ -2573,13 +2573,49 @@
def __init__(self, bug, level):
self.bug = bug
+ self.bugtask = None # Implies all.
assert level is not None
self.level = level
+ self.cache = {self.cache_key: self}
+ # This is often used in event handlers, many of which block implicit
+ # flushes. However, the data needs to be in the database for the
+ # queries herein to give correct answers.
+ Store.of(bug).flush()
+
+ @property
+ def cache_key(self):
+ bugtask_id = None if self.bugtask is None else self.bugtask.id
+ return self.bug.id, bugtask_id, self.level
+
+ def forTask(self, bugtask):
+ """Create a new `BugSubscriptionInfo` limited to `bugtask`.
+
+ The given task must refer to this object's bug. If `None` is passed a
+ new `BugSubscriptionInfo` instance is returned with no limit.
+ """
+ info = self.__class__(self.bug, self.level)
+ info.bugtask, info.cache = bugtask, self.cache
+ return self.cache.setdefault(info.cache_key, info)
+
+ def forLevel(self, level):
+ """Create a new `BugSubscriptionInfo` limited to `level`."""
+ info = self.__class__(self.bug, level)
+ info.bugtask, info.cache = self.bugtask, self.cache
+ return self.cache.setdefault(info.cache_key, info)
+
+ @cachedproperty
+ @freeze(BugSubscriberSet)
+ def muted_subscribers(self):
+ muted_people = Select(BugMute.person_id, BugMute.bug == self.bug)
+ return load_people(Person.id.is_in(muted_people))
@cachedproperty
@freeze(BugSubscriptionSet)
- def old_direct_subscriptions(self):
- """The bug's direct subscriptions."""
+ def direct_subscriptions(self):
+ """The bug's direct subscriptions.
+
+ Excludes muted subscriptions.
+ """
return IStore(BugSubscription).find(
BugSubscription,
BugSubscription.bug_notification_level >= self.level,
@@ -2587,34 +2623,33 @@
Not(In(BugSubscription.person_id,
Select(BugMute.person_id, BugMute.bug_id == self.bug.id))))
- @cachedproperty
- def direct_subscriptions_and_subscribers(self):
- """The bug's direct subscriptions."""
- res = IStore(BugSubscription).find(
- (BugSubscription, Person),
- BugSubscription.bug_notification_level >= self.level,
- BugSubscription.bug == self.bug,
- BugSubscription.person_id == Person.id,
- Not(In(BugSubscription.person_id,
- Select(BugMute.person_id,
- BugMute.bug_id == self.bug.id))))
- # Here we could test for res.count() but that will execute another
- # query. This structure avoids the extra query.
- return zip(*res) or ((), ())
-
- @cachedproperty
- @freeze(BugSubscriptionSet)
- def direct_subscriptions(self):
- return self.direct_subscriptions_and_subscribers[0]
-
- @cachedproperty
- @freeze(BugSubscriberSet)
+ @property
def direct_subscribers(self):
- return self.direct_subscriptions_and_subscribers[1]
+ """The bug's direct subscriptions.
+
+ Excludes muted subscribers.
+ """
+ return self.direct_subscriptions.subscribers
+
+ @property
+ def all_direct_subscriptions(self):
+ """The bug's direct subscriptions at all levels.
+
+ Excludes muted subscriptions.
+ """
+ return self.forLevel(
+ BugNotificationLevel.LIFECYCLE).direct_subscriptions
+
+ @property
+ def all_direct_subscribers(self):
+ """The bug's direct subscribers at all levels.
+
+ Excludes muted subscribers.
+ """
+ return self.all_direct_subscriptions.subscribers
@cachedproperty
def duplicate_subscriptions_and_subscribers(self):
- """Subscriptions to duplicates of the bug."""
if self.bug.private:
return ((), ())
else:
@@ -2633,20 +2668,28 @@
@cachedproperty
@freeze(BugSubscriptionSet)
def duplicate_subscriptions(self):
+ """Subscriptions to duplicates of the bug.
+
+ Excludes muted subscriptions.
+ """
return self.duplicate_subscriptions_and_subscribers[0]
@cachedproperty
@freeze(BugSubscriberSet)
def duplicate_subscribers(self):
+ """Subscribers to duplicates of the bug.
+
+ Excludes muted subscribers.
+ """
return self.duplicate_subscriptions_and_subscribers[1]
@cachedproperty
@freeze(BugSubscriptionSet)
def duplicate_only_subscriptions(self):
- """Subscriptions to duplicates of the bug.
+ """Subscriptions to duplicates of the bug only.
- Excludes subscriptions for people who have a direct subscription or
- are also notified for another reason.
+ Excludes muted subscriptions, subscriptions for people who have a
+ direct subscription, or who are also notified for another reason.
"""
self.duplicate_subscribers # Pre-load subscribers.
higher_precedence = (
@@ -2656,47 +2699,88 @@
subscription for subscription in self.duplicate_subscriptions
if subscription.person not in higher_precedence)
+ @property
+ def duplicate_only_subscribers(self):
+ """Subscribers to duplicates of the bug only.
+
+ Excludes muted subscribers, subscribers who have a direct
+ subscription, or who are also notified for another reason.
+ """
+ return self.duplicate_only_subscriptions.subscribers
+
@cachedproperty
@freeze(StructuralSubscriptionSet)
def structural_subscriptions(self):
- """Structural subscriptions to the bug's targets."""
- return list(get_structural_subscriptions_for_bug(self.bug))
+ """Structural subscriptions to the bug's targets.
+
+ Excludes direct subscriptions.
+ """
+ subject = self.bug if self.bugtask is None else self.bugtask
+ return get_structural_subscriptions(subject, self.level)
+
+ @property
+ def structural_subscribers(self):
+ """Structural subscribers to the bug's targets.
+
+ Excludes direct subscribers.
+ """
+ return self.structural_subscriptions.subscribers
@cachedproperty
@freeze(BugSubscriberSet)
def all_assignees(self):
- """Assignees of the bug's tasks."""
- assignees = Select(BugTask.assigneeID, BugTask.bug == self.bug)
- return load_people(Person.id.is_in(assignees))
+ """Assignees of the bug's tasks.
+
+ *Does not* exclude muted subscribers.
+ """
+ if self.bugtask is None:
+ assignees = Select(BugTask.assigneeID, BugTask.bug == self.bug)
+ return load_people(Person.id.is_in(assignees))
+ else:
+ return load_people(Person.id == self.bugtask.assigneeID)
@cachedproperty
@freeze(BugSubscriberSet)
def all_pillar_owners_without_bug_supervisors(self):
- """Owners of pillars for which no Bug supervisor is configured."""
- for bugtask in self.bug.bugtasks:
+ """Owners of pillars for which there is no bug supervisor.
+
+ The pillars must also use Launchpad for bug tracking.
+
+ *Does not* exclude muted subscribers.
+ """
+ if self.bugtask is None:
+ bugtasks = self.bug.bugtasks
+ else:
+ bugtasks = [self.bugtask]
+ for bugtask in bugtasks:
pillar = bugtask.pillar
- if pillar.bug_supervisor is None:
- yield pillar.owner
+ if pillar.official_malone:
+ if pillar.bug_supervisor is None:
+ yield pillar.owner
@cachedproperty
def also_notified_subscribers(self):
- """All subscribers except direct and dupe subscribers."""
+ """All subscribers except direct and dupe subscribers.
+
+ Excludes muted subscribers.
+ """
if self.bug.private:
return BugSubscriberSet()
else:
- muted = IStore(BugMute).find(
- Person,
- BugMute.person_id == Person.id,
- BugMute.bug == self.bug)
- return BugSubscriberSet().union(
- self.structural_subscriptions.subscribers,
+ subscribers = BugSubscriberSet().union(
+ self.structural_subscribers,
self.all_pillar_owners_without_bug_supervisors,
- self.all_assignees).difference(
- self.direct_subscribers).difference(muted)
+ self.all_assignees)
+ return subscribers.difference(
+ self.all_direct_subscribers,
+ self.muted_subscribers)
@cachedproperty
def indirect_subscribers(self):
- """All subscribers except direct subscribers."""
+ """All subscribers except direct subscribers.
+
+ Excludes muted subscribers.
+ """
return self.also_notified_subscribers.union(
self.duplicate_subscribers)
=== modified file 'lib/lp/bugs/model/structuralsubscription.py'
--- lib/lp/bugs/model/structuralsubscription.py 2011-12-30 06:14:56 +0000
+++ lib/lp/bugs/model/structuralsubscription.py 2012-01-03 17:52:00 +0000
@@ -3,10 +3,11 @@
__metaclass__ = type
__all__ = [
+ 'get_structural_subscribers',
+ 'get_structural_subscription_targets',
+ 'get_structural_subscriptions',
'get_structural_subscriptions_for_bug',
'get_structural_subscriptions_for_target',
- 'get_structural_subscribers',
- 'get_structural_subscription_targets',
'StructuralSubscription',
'StructuralSubscriptionTargetMixin',
]
@@ -588,54 +589,93 @@
*conditions)
-def get_structural_subscribers(
- bug_or_bugtask, recipients, level, direct_subscribers=None):
- """Return subscribers for bug or bugtask at level.
-
- :param bug: a bug.
- :param recipients: a BugNotificationRecipients object or None.
- Populates if given.
- :param level: a level from lp.bugs.enum.BugNotificationLevel.
- :param direct_subscribers: a collection of Person objects who are
- directly subscribed to the bug.
-
- Excludes structural subscriptions for people who are directly subscribed
- to the bug."""
- if IBug.providedBy(bug_or_bugtask):
- bug = bug_or_bugtask
- bugtasks = bug.bugtasks
- elif IBugTask.providedBy(bug_or_bugtask):
- bug = bug_or_bugtask.bug
- bugtasks = [bug_or_bugtask]
- else:
- raise ValueError('First argument must be bug or bugtask')
+def query_structural_subscriptions(
+ what, bug, bugtasks, level, exclude=None):
+ """Query into structural subscriptions for a given bug.
+
+ :param what: The fields to fetch. Choose from `Person`,
+ `StructuralSubscription`, `BugSubscriptionFilter`, or a combo.
+ :param bug: An `IBug`
+ :param bugtasks: An iterable of `IBugTask`.
+ :param level: A level from `BugNotificationLevel`.
+ :param exclude: `Person`s to exclude (e.g. direct subscribers).
+ """
filter_id_query = (
_get_structural_subscription_filter_id_query(
- bug, bugtasks, level, direct_subscribers))
+ bug, bugtasks, level, exclude))
if not filter_id_query:
return EmptyResultSet()
- # This is here because of a circular import.
- from lp.registry.model.person import Person
+ from lp.registry.model.person import Person # Circular.
source = IStore(StructuralSubscription).using(
StructuralSubscription,
Join(BugSubscriptionFilter,
BugSubscriptionFilter.structural_subscription_id ==
StructuralSubscription.id),
Join(Person,
- Person.id == StructuralSubscription.subscriberID),
- )
+ Person.id == StructuralSubscription.subscriberID))
+ conditions = In(
+ BugSubscriptionFilter.id, filter_id_query)
+ return source.find(what, conditions)
+
+
+def resolve_bug_and_bugtasks(bug_or_bugtask):
+ """Return a bug and a list of bugtasks given a bug or a bugtask.
+
+ :param bug_or_bugtask: An `IBug` or `IBugTask`.
+ :raises ValueError: If `bug_or_bugtask` does not provide `IBug` or
+ `IBugTask`.
+ """
+ if IBug.providedBy(bug_or_bugtask):
+ return bug_or_bugtask, bug_or_bugtask.bugtasks
+ elif IBugTask.providedBy(bug_or_bugtask):
+ return bug_or_bugtask.bug, [bug_or_bugtask]
+ else:
+ raise ValueError(
+ "Expected bug or bugtask, got %r" % (bug_or_bugtask,))
+
+
+def get_structural_subscriptions(bug_or_bugtask, level, exclude=None):
+ """Return subscriptions for bug or bugtask at level.
+
+ :param bug_or_bugtask: An `IBug` or `IBugTask`.
+ :param level: A level from `BugNotificationLevel`.
+ :param exclude: `Person`s to exclude (e.g. direct subscribers).
+ """
+ bug, bugtasks = resolve_bug_and_bugtasks(bug_or_bugtask)
+ subscriptions = query_structural_subscriptions(
+ StructuralSubscription, bug, bugtasks, level, exclude)
+ from lp.registry.model.person import Person # Circular.
+ # Return only the first subscription and filter per subscriber.
+ subscriptions.config(distinct=(Person.id,))
+ subscriptions.order_by(
+ Person.id, StructuralSubscription.id,
+ BugSubscriptionFilter.id)
+ return subscriptions
+
+
+def get_structural_subscribers(
+ bug_or_bugtask, recipients, level, exclude=None):
+ """Return subscribers for bug or bugtask at level.
+
+ :param bug_or_bugtask: An `IBug` or `IBugTask`.
+ :param recipients: A `BugNotificationRecipients` object or
+ `None`, which will be populated if provided.
+ :param level: A level from `BugNotificationLevel`.
+ :param exclude: `Person`s to exclude (e.g. direct subscribers).
+ """
+ from lp.registry.model.person import Person # Circular.
+ bug, bugtasks = resolve_bug_and_bugtasks(bug_or_bugtask)
if recipients is None:
- return source.find(
- Person,
- In(BugSubscriptionFilter.id,
- filter_id_query)).config(distinct=True).order_by()
+ subscribers = query_structural_subscriptions(
+ Person, bug, bugtasks, level, exclude)
+ subscribers.config(distinct=True)
+ return subscribers.order_by()
else:
+ results = query_structural_subscriptions(
+ (Person, StructuralSubscription, BugSubscriptionFilter),
+ bug, bugtasks, level, exclude)
subscribers = []
- query_results = source.find(
- (Person, StructuralSubscription, BugSubscriptionFilter),
- In(BugSubscriptionFilter.id, filter_id_query))
- for person, subscription, filter in query_results:
- # Set up results.
+ for person, subscription, filter in results:
if person not in recipients:
subscribers.append(person)
recipients.addStructuralSubscriber(
=== modified file 'lib/lp/bugs/model/tests/test_bug.py'
--- lib/lp/bugs/model/tests/test_bug.py 2012-01-01 02:58:52 +0000
+++ lib/lp/bugs/model/tests/test_bug.py 2012-01-03 17:52:00 +0000
@@ -22,6 +22,7 @@
from lp.bugs.errors import BugCannotBePrivate
from lp.bugs.interfaces.bugnotification import IBugNotificationSet
from lp.bugs.interfaces.bugtask import BugTaskStatus
+from lp.bugs.mail.bugnotificationrecipients import BugNotificationRecipients
from lp.bugs.model.bug import (
BugNotification,
BugSubscriptionInfo,
@@ -35,6 +36,7 @@
feature_flags,
login_person,
person_logged_in,
+ record_two_runs,
set_feature_flag,
StormStatementRecorder,
TestCaseWithFactory,
@@ -166,7 +168,7 @@
with StormStatementRecorder() as recorder:
subscribers = list(bug.getDirectSubscribers())
self.assertThat(len(subscribers), Equals(10 + 1))
- self.assertThat(recorder, HasQueryCount(Equals(1)))
+ self.assertThat(recorder, HasQueryCount(Equals(2)))
def test_mark_as_duplicate_query_count(self):
bug = self.factory.makeBug()
@@ -476,6 +478,75 @@
self.assertContentEqual(public_branches, linked_branches)
self.assertNotIn(private_branch, linked_branches)
+ def test_get_direct_subscribers_with_recipients_query_count(self):
+ # getDirectSubscribers() uses a constant number of queries when given
+ # a recipients argument regardless of the number of subscribers.
+ bug = self.factory.makeBug()
+
+ def create_subscriber():
+ subscriber = self.factory.makePerson()
+ with person_logged_in(subscriber):
+ bug.subscribe(subscriber, subscriber)
+
+ def get_subscribers():
+ recipients = BugNotificationRecipients()
+ bug.getDirectSubscribers(recipients=recipients)
+
+ recorder1, recorder2 = record_two_runs(
+ get_subscribers, create_subscriber, 3)
+ self.assertThat(
+ recorder2, HasQueryCount(Equals(recorder1.count)))
+
+ def test_get_subscribers_from_dupes_with_recipients_query_count(self):
+ # getSubscribersFromDuplicates() uses a constant number of queries
+ # when given a recipients argument regardless of the number of
+ # subscribers.
+ bug = self.factory.makeBug()
+ duplicate_bug = self.factory.makeBug()
+ with person_logged_in(duplicate_bug.owner):
+ duplicate_bug.markAsDuplicate(bug)
+
+ def create_subscriber():
+ subscriber = self.factory.makePerson()
+ with person_logged_in(subscriber):
+ duplicate_bug.subscribe(subscriber, subscriber)
+
+ def get_subscribers():
+ recipients = BugNotificationRecipients()
+ bug.getSubscribersFromDuplicates(recipients=recipients)
+
+ recorder1, recorder2 = record_two_runs(
+ get_subscribers, create_subscriber, 3)
+ self.assertThat(
+ recorder2, HasQueryCount(Equals(recorder1.count)))
+
+ def test_get_also_notified_subscribers_with_recipients_query_count(self):
+ # getAlsoNotifiedSubscribers() uses a constant number of queries when
+ # given a recipients argument regardless of the number of subscribers.
+ bug = self.factory.makeBug()
+
+ def create_stuff():
+ # Create a new bugtask, set its assignee, set its pillar's
+ # official_malone=True, and subscribe someone to its target.
+ bugtask = self.factory.makeBugTask(bug=bug)
+ with person_logged_in(bugtask.owner):
+ bugtask.transitionToAssignee(bugtask.owner)
+ with person_logged_in(bugtask.pillar.owner):
+ bugtask.pillar.official_malone = True
+ subscriber = self.factory.makePerson()
+ with person_logged_in(subscriber):
+ bugtask.target.addSubscription(
+ subscriber, subscriber)
+
+ def get_subscribers():
+ recipients = BugNotificationRecipients()
+ bug.getAlsoNotifiedSubscribers(recipients=recipients)
+
+ recorder1, recorder2 = record_two_runs(
+ get_subscribers, create_stuff, 3)
+ self.assertThat(
+ recorder2, HasQueryCount(Equals(recorder1.count)))
+
class TestBugPrivateAndSecurityRelatedUpdatesMixin:
=== modified file 'lib/lp/bugs/model/tests/test_bugsubscriptioninfo.py'
--- lib/lp/bugs/model/tests/test_bugsubscriptioninfo.py 2012-01-01 02:58:52 +0000
+++ lib/lp/bugs/model/tests/test_bugsubscriptioninfo.py 2012-01-03 17:52:00 +0000
@@ -128,9 +128,8 @@
with person_logged_in(self.bug.owner):
self.bug.unsubscribe(self.bug.owner, self.bug.owner)
- def getInfo(self):
- return BugSubscriptionInfo(
- self.bug, BugNotificationLevel.LIFECYCLE)
+ def getInfo(self, level=BugNotificationLevel.LIFECYCLE):
+ return BugSubscriptionInfo(self.bug, level)
def _create_direct_subscriptions(self):
subscribers = (
@@ -142,16 +141,75 @@
for subscriber in subscribers)
return subscribers, subscriptions
+ def test_forTask(self):
+ # `forTask` returns a new `BugSubscriptionInfo` narrowed to the given
+ # bugtask.
+ info = self.getInfo()
+ self.assertIs(None, info.bugtask)
+ # If called with the current bugtask the same `BugSubscriptionInfo`
+ # instance is returned.
+ self.assertIs(info, info.forTask(info.bugtask))
+ # If called with a different bugtask a new `BugSubscriptionInfo` is
+ # created.
+ bugtask = self.bug.default_bugtask
+ info_for_task = info.forTask(bugtask)
+ self.assertIs(bugtask, info_for_task.bugtask)
+ self.assertIsNot(info, info_for_task)
+ # The instances share a cache of `BugSubscriptionInfo` instances.
+ expected_cache = {
+ info.cache_key: info,
+ info_for_task.cache_key: info_for_task,
+ }
+ self.assertEqual(expected_cache, info.cache)
+ self.assertIs(info.cache, info_for_task.cache)
+ # Calling `forTask` again looks in the cache first.
+ self.assertIs(info, info_for_task.forTask(info.bugtask))
+ self.assertIs(info_for_task, info.forTask(info_for_task.bugtask))
+ # The level is the same.
+ self.assertEqual(info.level, info_for_task.level)
+
+ def test_forLevel(self):
+ # `forLevel` returns a new `BugSubscriptionInfo` narrowed to the given
+ # subscription level.
+ info = self.getInfo()
+ # If called with the current level the same `BugSubscriptionInfo`
+ # instance is returned.
+ self.assertIs(info, info.forLevel(info.level))
+ # If called with a different level a new `BugSubscriptionInfo` is
+ # created.
+ level = BugNotificationLevel.METADATA
+ info_for_level = info.forLevel(level)
+ self.assertEqual(level, info_for_level.level)
+ self.assertIsNot(info, info_for_level)
+ # The instances share a cache of `BugSubscriptionInfo` instances.
+ expected_cache = {
+ info.cache_key: info,
+ info_for_level.cache_key: info_for_level,
+ }
+ self.assertEqual(expected_cache, info.cache)
+ self.assertIs(info.cache, info_for_level.cache)
+ # Calling `forLevel` again looks in the cache first.
+ self.assertIs(info, info_for_level.forLevel(info.level))
+ self.assertIs(info_for_level, info.forLevel(info_for_level.level))
+ # The bugtask is the same.
+ self.assertIs(info.bugtask, info_for_level.bugtask)
+
+ def test_muted(self):
+ # The set of muted subscribers for the bug.
+ subscribers, subscriptions = self._create_direct_subscriptions()
+ sub1, sub2 = subscribers
+ with person_logged_in(sub1):
+ self.bug.mute(sub1, sub1)
+ self.assertEqual(set([sub1]), self.getInfo().muted_subscribers)
+
def test_direct(self):
# The set of direct subscribers.
subscribers, subscriptions = self._create_direct_subscriptions()
found_subscriptions = self.getInfo().direct_subscriptions
self.assertEqual(set(subscriptions), found_subscriptions)
- self.assertEqual(subscriptions, found_subscriptions.sorted)
self.assertEqual(set(subscribers), found_subscriptions.subscribers)
- self.assertEqual(subscribers, found_subscriptions.subscribers.sorted)
- def test_muted_direct(self):
+ def test_direct_muted(self):
# If a direct is muted, it is not listed.
subscribers, subscriptions = self._create_direct_subscriptions()
with person_logged_in(subscribers[0]):
@@ -159,6 +217,17 @@
found_subscriptions = self.getInfo().direct_subscriptions
self.assertEqual(set([subscriptions[1]]), found_subscriptions)
+ def test_all_direct(self):
+ # The set of all direct subscribers, regardless of level.
+ subscribers, subscriptions = self._create_direct_subscriptions()
+ # Change the first subscription to be for comments only.
+ sub1, sub2 = subscriptions
+ with person_logged_in(sub1.person):
+ sub1.bug_notification_level = BugNotificationLevel.LIFECYCLE
+ info = self.getInfo(BugNotificationLevel.COMMENTS)
+ self.assertEqual(set((sub2,)), info.direct_subscriptions)
+ self.assertEqual(set((sub1, sub2)), info.all_direct_subscriptions)
+
def _create_duplicate_subscription(self):
duplicate_bug = self.factory.makeBug(product=self.target)
with person_logged_in(duplicate_bug.owner):
@@ -172,9 +241,7 @@
# The set of subscribers from duplicate bugs.
found_subscriptions = self.getInfo().duplicate_subscriptions
self.assertEqual(set(), found_subscriptions)
- self.assertEqual((), found_subscriptions.sorted)
self.assertEqual(set(), found_subscriptions.subscribers)
- self.assertEqual((), found_subscriptions.subscribers.sorted)
duplicate_bug, duplicate_bug_subscription = (
self._create_duplicate_subscription())
found_subscriptions = self.getInfo().duplicate_subscriptions
@@ -182,16 +249,10 @@
set([duplicate_bug_subscription]),
found_subscriptions)
self.assertEqual(
- (duplicate_bug_subscription,),
- found_subscriptions.sorted)
- self.assertEqual(
set([duplicate_bug.owner]),
found_subscriptions.subscribers)
- self.assertEqual(
- (duplicate_bug.owner,),
- found_subscriptions.subscribers.sorted)
- def test_muted_duplicate(self):
+ def test_duplicate_muted(self):
# If a duplicate is muted, it is not listed.
duplicate_bug, duplicate_bug_subscription = (
self._create_duplicate_subscription())
@@ -210,9 +271,7 @@
self.bug.setPrivate(True, self.bug.owner)
found_subscriptions = self.getInfo().duplicate_subscriptions
self.assertEqual(set(), found_subscriptions)
- self.assertEqual((), found_subscriptions.sorted)
self.assertEqual(set(), found_subscriptions.subscribers)
- self.assertEqual((), found_subscriptions.subscribers.sorted)
def test_duplicate_only(self):
# The set of duplicate subscriptions where the subscriber has no other
@@ -236,32 +295,61 @@
found_subscriptions = self.getInfo().duplicate_only_subscriptions
self.assertEqual(set(), found_subscriptions)
- def test_structural(self):
+ def test_structural_subscriptions(self):
+ # The set of structural subscriptions.
+ subscribers = (
+ self.factory.makePerson(),
+ self.factory.makePerson())
+ with person_logged_in(self.bug.owner):
+ subscriptions = tuple(
+ self.target.addBugSubscription(subscriber, subscriber)
+ for subscriber in subscribers)
+ found_subscriptions = self.getInfo().structural_subscriptions
+ self.assertEqual(set(subscriptions), found_subscriptions)
+
+ def test_structural_subscriptions_muted(self):
+ # The set of structural subscriptions DOES NOT exclude muted
+ # subscriptions.
+ subscriber = self.factory.makePerson()
+ with person_logged_in(subscriber):
+ self.bug.mute(subscriber, subscriber)
+ with person_logged_in(self.bug.owner):
+ subscription = self.target.addBugSubscription(
+ subscriber, subscriber)
+ found_subscriptions = self.getInfo().structural_subscriptions
+ self.assertEqual(set([subscription]), found_subscriptions)
+
+ def test_structural_subscribers(self):
# The set of structural subscribers.
subscribers = (
self.factory.makePerson(),
self.factory.makePerson())
with person_logged_in(self.bug.owner):
- subscriptions = tuple(
+ for subscriber in subscribers:
self.target.addBugSubscription(subscriber, subscriber)
- for subscriber in subscribers)
- found_subscriptions = self.getInfo().structural_subscriptions
- self.assertEqual(set(subscriptions), found_subscriptions)
- self.assertEqual(subscriptions, found_subscriptions.sorted)
- self.assertEqual(set(subscribers), found_subscriptions.subscribers)
- self.assertEqual(subscribers, found_subscriptions.subscribers.sorted)
+ found_subscribers = self.getInfo().structural_subscribers
+ self.assertEqual(set(subscribers), found_subscribers)
+
+ def test_structural_subscribers_muted(self):
+ # The set of structural subscribers DOES NOT exclude muted
+ # subscribers.
+ subscriber = self.factory.makePerson()
+ with person_logged_in(subscriber):
+ self.bug.mute(subscriber, subscriber)
+ with person_logged_in(self.bug.owner):
+ self.target.addBugSubscription(subscriber, subscriber)
+ found_subscribers = self.getInfo().structural_subscribers
+ self.assertEqual(set([subscriber]), found_subscribers)
def test_all_assignees(self):
# The set of bugtask assignees for bugtasks that have been assigned.
found_assignees = self.getInfo().all_assignees
self.assertEqual(set(), found_assignees)
- self.assertEqual((), found_assignees.sorted)
bugtask = self.bug.default_bugtask
with person_logged_in(bugtask.pillar.bug_supervisor):
bugtask.transitionToAssignee(self.bug.owner)
found_assignees = self.getInfo().all_assignees
self.assertEqual(set([self.bug.owner]), found_assignees)
- self.assertEqual((self.bug.owner,), found_assignees.sorted)
bugtask2 = self.factory.makeBugTask(bug=self.bug)
with person_logged_in(bugtask2.pillar.owner):
bugtask2.transitionToAssignee(bugtask2.owner)
@@ -269,36 +357,50 @@
self.assertEqual(
set([self.bug.owner, bugtask2.owner]),
found_assignees)
+ # Getting info for a specific bugtask will return the assignee for
+ # that bugtask only.
self.assertEqual(
- (self.bug.owner, bugtask2.owner),
- found_assignees.sorted)
+ set([bugtask2.owner]),
+ self.getInfo().forTask(bugtask2).all_assignees)
def test_all_pillar_owners_without_bug_supervisors(self):
# The set of owners of pillars for which no bug supervisor is
- # configured.
+ # configured and which use Launchpad for bug tracking.
[bugtask] = self.bug.bugtasks
found_owners = (
self.getInfo().all_pillar_owners_without_bug_supervisors)
self.assertEqual(set(), found_owners)
- self.assertEqual((), found_owners.sorted)
# Clear the supervisor for the first bugtask's target.
with person_logged_in(bugtask.target.owner):
bugtask.target.setBugSupervisor(None, bugtask.owner)
+ bugtask.pillar.official_malone = False
+ # The pillar does not use Launchpad, so the collection is still empty.
+ found_owners = (
+ self.getInfo().all_pillar_owners_without_bug_supervisors)
+ self.assertEqual(set(), found_owners)
+ # When a pillar does use Launchpad the collection includes the
+ # pillar's owner.
+ with person_logged_in(bugtask.target.owner):
+ bugtask.pillar.official_malone = True
found_owners = (
self.getInfo().all_pillar_owners_without_bug_supervisors)
self.assertEqual(set([bugtask.pillar.owner]), found_owners)
- self.assertEqual((bugtask.pillar.owner,), found_owners.sorted)
- # Add another bugtask with a bug supervisor.
- target2 = self.factory.makeProduct(bug_supervisor=None)
+ # Add another bugtask for a pillar that uses Launchpad but without a
+ # bug supervisor.
+ target2 = self.factory.makeProduct(
+ bug_supervisor=None, official_malone=True)
bugtask2 = self.factory.makeBugTask(bug=self.bug, target=target2)
found_owners = (
self.getInfo().all_pillar_owners_without_bug_supervisors)
self.assertEqual(
set([bugtask.pillar.owner, bugtask2.pillar.owner]),
found_owners)
+ # Getting subscription info for just a specific bugtask will yield
+ # owners for only the pillar associated with that bugtask.
+ info_for_bugtask2 = self.getInfo().forTask(bugtask2)
self.assertEqual(
- (bugtask.pillar.owner, bugtask2.pillar.owner),
- found_owners.sorted)
+ set([bugtask2.pillar.owner]),
+ info_for_bugtask2.all_pillar_owners_without_bug_supervisors)
def _create_also_notified_subscribers(self):
# Add an assignee, a bug supervisor and a structural subscriber.
@@ -319,7 +421,6 @@
# The set of also notified subscribers.
found_subscribers = self.getInfo().also_notified_subscribers
self.assertEqual(set(), found_subscribers)
- self.assertEqual((), found_subscribers.sorted)
assignee, supervisor, structural_subscriber = (
self._create_also_notified_subscribers())
# Add a direct subscription.
@@ -332,11 +433,8 @@
self.assertEqual(
set([assignee, supervisor, structural_subscriber]),
found_subscribers)
- self.assertEqual(
- (assignee, supervisor, structural_subscriber),
- found_subscribers.sorted)
- def test_muted_also_notified_subscribers(self):
+ def test_also_notified_subscribers_muted(self):
# If someone is muted, they are not listed in the
# also_notified_subscribers.
assignee, supervisor, structural_subscriber = (
@@ -371,7 +469,6 @@
self.bug.setPrivate(True, self.bug.owner)
found_subscribers = self.getInfo().also_notified_subscribers
self.assertEqual(set(), found_subscribers)
- self.assertEqual((), found_subscribers.sorted)
def test_indirect_subscribers(self):
# The set of indirect subscribers is the union of also notified
@@ -387,9 +484,6 @@
self.assertEqual(
set([assignee, duplicate_bug.owner]),
found_subscribers)
- self.assertEqual(
- (assignee, duplicate_bug.owner),
- found_subscribers.sorted)
class TestBugSubscriptionInfoPermissions(TestCaseWithFactory):
@@ -444,27 +538,28 @@
yield recorder
self.assertThat(recorder, condition)
- def exercise_subscription_set(self, set_name):
+ def exercise_subscription_set(self, set_name, counts=(1, 1, 0)):
# Looking up subscriptions takes a single query.
- with self.exactly_x_queries(1):
+ with self.exactly_x_queries(counts[0]):
getattr(self.info, set_name)
# Getting the subscribers results in one additional query.
- with self.exactly_x_queries(1):
+ with self.exactly_x_queries(counts[1]):
getattr(self.info, set_name).subscribers
# Everything is now cached so no more queries are needed.
- with self.exactly_x_queries(0):
+ with self.exactly_x_queries(counts[2]):
getattr(self.info, set_name).subscribers
getattr(self.info, set_name).subscribers.sorted
- def exercise_subscription_set_sorted_first(self, set_name):
+ def exercise_subscription_set_sorted_first(
+ self, set_name, counts=(1, 1, 0)):
# Looking up subscriptions takes a single query.
- with self.exactly_x_queries(1):
+ with self.exactly_x_queries(counts[0]):
getattr(self.info, set_name)
# Getting the sorted subscriptions takes one additional query.
- with self.exactly_x_queries(1):
+ with self.exactly_x_queries(counts[1]):
getattr(self.info, set_name).sorted
# Everything is now cached so no more queries are needed.
- with self.exactly_x_queries(0):
+ with self.exactly_x_queries(counts[2]):
getattr(self.info, set_name).subscribers
getattr(self.info, set_name).subscribers.sorted
@@ -476,6 +571,10 @@
self.exercise_subscription_set_sorted_first(
"direct_subscriptions")
+ def test_all_direct_subscriptions(self):
+ self.exercise_subscription_set(
+ "all_direct_subscriptions")
+
def make_duplicate_bug(self):
duplicate_bug = self.factory.makeBug(product=self.target)
with person_logged_in(duplicate_bug.owner):
@@ -501,18 +600,19 @@
self.info.duplicate_subscriptions.subscribers
def add_structural_subscriber(self):
- with person_logged_in(self.bug.owner):
- self.target.addSubscription(self.bug.owner, self.bug.owner)
+ subscriber = self.factory.makePerson()
+ with person_logged_in(subscriber):
+ self.target.addSubscription(subscriber, subscriber)
def test_structural_subscriptions(self):
self.add_structural_subscriber()
self.exercise_subscription_set(
- "structural_subscriptions")
+ "structural_subscriptions", (2, 1, 0))
def test_structural_subscriptions_sorted_first(self):
self.add_structural_subscriber()
self.exercise_subscription_set_sorted_first(
- "structural_subscriptions")
+ "structural_subscriptions", (2, 1, 0))
def test_all_assignees(self):
with self.exactly_x_queries(1):
@@ -522,8 +622,8 @@
# Getting all bug supervisors and pillar owners can take several
# queries. However, there are typically few tasks so the trade for
# simplicity of implementation is acceptable. Only the simplest case
- # is tested here.
- with self.exactly_x_queries(1):
+ # is tested here (everything is already cached).
+ with self.exactly_x_queries(0):
self.info.all_pillar_owners_without_bug_supervisors
def test_also_notified_subscribers(self):
@@ -536,7 +636,7 @@
self.info.all_assignees
self.info.all_pillar_owners_without_bug_supervisors
self.info.direct_subscriptions.subscribers
- self.info.structural_subscriptions.subscribers
+ self.info.structural_subscribers
with self.exactly_x_queries(1):
self.info.also_notified_subscribers
=== modified file 'lib/lp/bugs/tests/test_structuralsubscription.py'
--- lib/lp/bugs/tests/test_structuralsubscription.py 2012-01-01 02:58:52 +0000
+++ lib/lp/bugs/tests/test_structuralsubscription.py 2012-01-03 17:52:00 +0000
@@ -27,8 +27,10 @@
from lp.bugs.model.structuralsubscription import (
get_structural_subscribers,
get_structural_subscription_targets,
+ get_structural_subscriptions,
get_structural_subscriptions_for_bug,
)
+from lp.services.database.decoratedresultset import DecoratedResultSet
from lp.testing import (
anonymous_logged_in,
login_person,
@@ -42,6 +44,9 @@
)
+RESULT_SETS = ResultSet, EmptyResultSet, DecoratedResultSet
+
+
class TestStructuralSubscription(TestCaseWithFactory):
layer = DatabaseFunctionalLayer
@@ -634,6 +639,99 @@
self.assertEqual(set([self_sub]), set(subscriptions))
+class TestGetStructuralSubscriptions(TestCaseWithFactory):
+
+ layer = DatabaseFunctionalLayer
+
+ def make_product_with_bug(self):
+ product = self.factory.makeProduct()
+ bug = self.factory.makeBug(product=product)
+ return product, bug
+
+ def test_get_structural_subscriptions_no_subscriptions(self):
+ # If there are no subscriptions for any of the bug's targets then no
+ # subscriptions will be returned by get_structural_subscriptions().
+ product, bug = self.make_product_with_bug()
+ subscriptions = get_structural_subscriptions(bug, None)
+ self.assertIsInstance(subscriptions, RESULT_SETS)
+ self.assertEqual([], list(subscriptions))
+
+ def test_get_structural_subscriptions_single_target(self):
+ # Subscriptions for any of the bug's targets are returned.
+ subscriber = self.factory.makePerson()
+ login_person(subscriber)
+ product, bug = self.make_product_with_bug()
+ subscription = product.addBugSubscription(subscriber, subscriber)
+ self.assertContentEqual(
+ [subscription], get_structural_subscriptions(bug, None))
+
+ def test_get_structural_subscriptions_multiple_targets(self):
+ # Subscriptions 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)
+ subscription1 = product1.addBugSubscription(subscriber1, subscriber1)
+ product2 = self.factory.makeProduct(owner=actor)
+ subscription2 = product2.addBugSubscription(subscriber2, subscriber2)
+
+ bug = self.factory.makeBug(product=product1)
+ bug.addTask(actor, product2)
+
+ subscriptions = get_structural_subscriptions(bug, None)
+ self.assertIsInstance(subscriptions, RESULT_SETS)
+ self.assertContentEqual(
+ [subscription1, subscription2], subscriptions)
+
+ def test_get_structural_subscriptions_multiple_targets_2(self):
+ # Only the first of multiple subscriptions for a person is returned
+ # when they have multiple matching subscriptions.
+ actor = self.factory.makePerson()
+ login_person(actor)
+
+ subscriber = self.factory.makePerson()
+ product1 = self.factory.makeProduct(owner=actor)
+ subscription1 = product1.addBugSubscription(subscriber, subscriber)
+ product2 = self.factory.makeProduct(owner=actor)
+ product2.addBugSubscription(subscriber, subscriber)
+
+ bug = self.factory.makeBug(product=product1)
+ bug.addTask(actor, product2)
+
+ subscriptions = get_structural_subscriptions(bug, None)
+ self.assertIsInstance(subscriptions, RESULT_SETS)
+ self.assertContentEqual([subscription1], subscriptions)
+
+ def test_get_structural_subscriptions_level(self):
+ # get_structural_subscriptions() respects the given level.
+ subscriber = self.factory.makePerson()
+ login_person(subscriber)
+ product, bug = self.make_product_with_bug()
+ subscription = product.addBugSubscription(subscriber, subscriber)
+ filter = subscription.bug_filters.one()
+ filter.bug_notification_level = BugNotificationLevel.METADATA
+ self.assertContentEqual(
+ [subscription], get_structural_subscriptions(
+ bug, BugNotificationLevel.METADATA))
+ self.assertContentEqual(
+ [], get_structural_subscriptions(
+ bug, BugNotificationLevel.COMMENTS))
+
+ def test_get_structural_subscriptions_exclude(self):
+ # Subscriptions for any of the given excluded subscribers are not
+ # returned.
+ subscriber = self.factory.makePerson()
+ login_person(subscriber)
+ product, bug = self.make_product_with_bug()
+ product.addBugSubscription(subscriber, subscriber)
+ self.assertContentEqual(
+ [], get_structural_subscriptions(
+ bug, None, exclude=[subscriber]))
+
+
class TestGetStructuralSubscribers(TestCaseWithFactory):
layer = DatabaseFunctionalLayer
@@ -648,7 +746,7 @@
# subscribers will be returned by get_structural_subscribers().
product, bug = self.make_product_with_bug()
subscribers = get_structural_subscribers(bug, None, None, None)
- self.assertIsInstance(subscribers, (ResultSet, EmptyResultSet))
+ self.assertIsInstance(subscribers, RESULT_SETS)
self.assertEqual([], list(subscribers))
def test_getStructuralSubscribers_single_target(self):
@@ -678,7 +776,7 @@
bug.addTask(actor, product2)
subscribers = get_structural_subscribers(bug, None, None, None)
- self.assertIsInstance(subscribers, ResultSet)
+ self.assertIsInstance(subscribers, RESULT_SETS)
self.assertEqual(set([subscriber1, subscriber2]), set(subscribers))
def test_getStructuralSubscribers_recipients(self):
Follow ups
