launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #03575
[Merge] lp:~danilo/launchpad/proper-bug-muting into lp:launchpad/db-devel
Данило Шеган has proposed merging lp:~danilo/launchpad/proper-bug-muting into lp:launchpad/db-devel.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers): code
Stuart Bishop (stub): db
Related bugs:
Bug #772763 in Launchpad itself: "Unmuting a bug's notifications should restore your previous direct subscription"
https://bugs.launchpad.net/launchpad/+bug/772763
For more details, see:
https://code.launchpad.net/~danilo/launchpad/proper-bug-muting/+merge/60615
= Bug 772763: step 1 =
At the moment, muting a bug is implemented using BugSubscription by setting bug_notification_level on it to NOTHING. That has a bad side-effect that once someone decides to unmute their subscription, we can't restore the previous one. UI is also confusing ("unmuting" just removes the "mute", without actually re-subscribing you).
== Proposed fix ==
To parallel BugSubscriptionFilterMute, which allows one to mute a structural subscription filter, we implement a BugMute table which allows a single person to mute all email for a bug.
Since we only "expand" subscribers at a very late stage when emails are constructed (in construct_email_notifications), we can either filter muted recipients in there or in getRecipientFilterData which is used by this method with expanded recipients passed in. I did the change in getRecipientFilterData because it seems more sane to have all of the direct DB access in a single place. It's also more easily unit-tested.
I wonder if I should do the migration of NOTHING subscriptions to BugMute records in the DB patch or not. That depends on how many of them there are, but it'd probably be best to do it that way.
== Implementation details ==
* We provide a very basic BugMute table that is modelled after the BugSubscriptionFilterMute table. Bug muting/unmuting is modified to use it, and test is updated to match the new behaviour. This is all in model/bug.py, interfaces/bug.py and test_bug.py
* XXXes are there to remind me to talk to the team if we really want to log bug muting/unmuting in the bug activity log (assuming bug.addChange does that), as it used to with the use of subscribe/unsubscribe. I assume we'll just end up removing the XXXes.
* We modify getRecipientFilterData to accept a bug as the parameter as well, and fetch all muted subscribers so we can easily filter them out.
* Test in test_bugchanges.py is the integration test (interestingly, one that even failed for the existing implementation, because muting didn't work for all cases).
* UI is not modified in any way in this branch, which means that it will be a bit confusing; that's left for follow-up branches.
* Another branch will go on with removing the NOTHING level if possible.
* Some typos in registry/model/person.py and bugsubscriptionfilter.py are fixed along the way
* I have another 144 diff of lint fixes for mostly things not caused by my changes, not including it so the branch is smaller; similar holds for sampledata updates, I'll commit them after review.
== Tests ==
bin/test -cvvt TestBugSubscriptionMethods -t getRecipientFilterData -t test_description_changed_no_muted_email
== Demo and Q/A ==
With 'malone.advanced-subscriptions.enabled default 1 on' set as the feature flag, try playing with muting/unmuting (and seeing how your bug subscription gets restored to appropriate "level") on any BugTask:+index page (i.e. a bug page).
Note that until UI is fully adapted as well, you might need to refresh the page between actions.
= Launchpad lint =
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/bugs/model/bugnotification.py
database/schema/patch-2208-97-0.sql
lib/lp/bugs/configure.zcml
database/schema/comments.sql
lib/lp/bugs/tests/test_bugnotification.py
database/schema/security.cfg
lib/lp/bugs/model/bugsubscriptionfilter.py
lib/lp/bugs/scripts/tests/test_bugnotification.py
lib/lp/bugs/model/bug.py
lib/lp/bugs/tests/test_bugchanges.py
lib/lp/bugs/scripts/bugnotification.py
lib/lp/bugs/tests/test_bug.py
lib/lp/registry/model/person.py
lib/lp/bugs/interfaces/bugnotification.py
lib/lp/bugs/interfaces/bug.py
== Schema ==
database/sampledata/current.sql
database/sampledata/lintdata.sql differs from database/sampledata/current.sql.
Patches to the schema, or manual edits to database/sampledata/current.sql
do not match the dump of the launchpad_ftest_template database.
If database/sampledata/lintdata.sql is correct, copy it to
database/sampledata/current.sql.
Otherwise update database/sampledata/current.sql and run:
make schema
make newsampledata
cd database/sampledata
cp newsampledata.sql database/sampledata/current.sql
Run make schema again to update the test/dev database.
database/sampledata/current.sql
database/sampledata/lintdata-dev.sql differs from database/sampledata/current-dev.sql.
Patches to the schema, or manual edits to database/sampledata/current-dev.sql
do not match the dump of the launchpad_dev_template database.
If database/sampledata/lintdata-dev.sql is correct, copy it to
database/sampledata/current-dev.sql.
Otherwise update database/sampledata/current-dev.sql and run:
make schema
make newsampledata
cd database/sampledata
cp newsampledata-dev.sql database/sampledata/current-dev.sql
Run make schema again to update the test/dev database.
./lib/lp/bugs/model/bugnotification.py
147: local variable 'cached_people' is assigned to but never used
152: local variable 'cached_bugs' is assigned to but never used
273: E501 line too long (81 characters)
192: Line exceeds 78 characters.
273: Line exceeds 78 characters.
./lib/lp/bugs/tests/test_bugnotification.py
387: E201 whitespace after '{'
./lib/lp/bugs/scripts/tests/test_bugnotification.py
175: Line exceeds 78 characters.
./lib/lp/bugs/tests/test_bugchanges.py
211: local variable 'bug_subscription' is assigned to but never used
605: local variable 'old_tags' is assigned to but never used
629: local variable 'old_tags' is assigned to but never used
1020: local variable 'lifecycle_subscriber' is assigned to but never used
1742: local variable 'old_description' is assigned to but never used
1759: local variable 'old_description' is assigned to but never used
1774: local variable 'old_description' is assigned to but never used
1788: local variable 'old_description' is assigned to but never used
./lib/lp/registry/model/person.py
3849: local variable 'karma_total' is assigned to but never used
3573: W291 trailing whitespace
3575: W291 trailing whitespace
3577: W291 trailing whitespace
3579: W291 trailing whitespace
3581: W291 trailing whitespace
3598: W291 trailing whitespace
3573: Line has trailing whitespace.
3575: Line has trailing whitespace.
3577: Line has trailing whitespace.
3579: Line has trailing whitespace.
3581: Line has trailing whitespace.
3598: Line has trailing whitespace.
--
https://code.launchpad.net/~danilo/launchpad/proper-bug-muting/+merge/60615
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~danilo/launchpad/proper-bug-muting into lp:launchpad/db-devel.
=== modified file 'database/schema/comments.sql'
--- database/schema/comments.sql 2011-05-09 14:17:04 +0000
+++ database/schema/comments.sql 2011-05-11 11:47:01 +0000
@@ -190,6 +190,12 @@
COMMENT ON COLUMN BugJob.job_type IS 'The type of job (enumeration value). Allows us to query the database for a given subset of BugJobs.';
COMMENT ON COLUMN BugJob.json_data IS 'A JSON struct containing data for the job.';
+-- BugMute
+COMMENT ON TABLE BugMute IS 'Mutes for bug notifications.';
+COMMENT ON COLUMN BugMute.person IS 'The person that muted all notifications from this bug.';
+COMMENT ON COLUMN BugMute.bug IS 'The bug of this record';
+COMMENT ON COLUMN BugMute.date_created IS 'The date at which this mute was created.';
+
-- BugNomination
COMMENT ON TABLE BugNomination IS 'A bug nominated for fixing in a distroseries or productseries';
COMMENT ON COLUMN BugNomination.bug IS 'The bug being nominated.';
=== added file 'database/schema/patch-2208-97-0.sql'
--- database/schema/patch-2208-97-0.sql 1970-01-01 00:00:00 +0000
+++ database/schema/patch-2208-97-0.sql 2011-05-11 11:47:01 +0000
@@ -0,0 +1,24 @@
+-- Copyright 2011 Canonical Ltd. This software is licensed under the
+-- GNU Affero General Public License version 3 (see the file LICENSE).
+
+SET client_min_messages=ERROR;
+
+-- A table to store bug mutes in.
+
+CREATE TABLE BugMute (
+ person integer REFERENCES Person(id)
+ ON DELETE CASCADE NOT NULL,
+ bug integer REFERENCES Bug(id)
+ ON DELETE CASCADE NOT NULL,
+ date_created timestamp without time zone
+ DEFAULT timezone('UTC'::text, now()) NOT NULL,
+ CONSTRAINT bugmute_pkey PRIMARY KEY (person, bug)
+);
+
+-- We don't need an index on person, as the primary key index can be used
+-- for those lookups. We have an index on just the bug, as the bulk of our
+-- lookups will be on bugs.
+CREATE INDEX bugmute__bug__idx
+ ON BugMute(bug);
+
+INSERT INTO LaunchpadDatabaseRevision VALUES (2208, 97, 0);
=== modified file 'database/schema/security.cfg'
--- database/schema/security.cfg 2011-05-11 05:39:36 +0000
+++ database/schema/security.cfg 2011-05-11 11:47:01 +0000
@@ -1062,6 +1062,7 @@
public.bugattachment = SELECT, INSERT, UPDATE
public.bugjob = SELECT, INSERT
public.bugmessage = SELECT, INSERT, UPDATE
+public.bugmute = SELECT, INSERT, UPDATE, DELETE
public.bugnomination = SELECT, INSERT, UPDATE
public.bugpackageinfestation = SELECT, INSERT, UPDATE
public.bugproductinfestation = SELECT, INSERT, UPDATE
@@ -1445,6 +1446,7 @@
public.bugattachment = SELECT
public.bugjob = SELECT, INSERT
public.bugmessage = SELECT, INSERT
+public.bugmute = SELECT
public.bugnomination = SELECT
public.bugnotification = SELECT, INSERT, UPDATE
public.bugnotificationfilter = SELECT, INSERT
=== modified file 'lib/lp/bugs/configure.zcml'
--- lib/lp/bugs/configure.zcml 2011-04-13 18:48:42 +0000
+++ lib/lp/bugs/configure.zcml 2011-05-11 11:47:01 +0000
@@ -645,6 +645,13 @@
set_schema=".interfaces.bugsubscriptionfilter.IBugSubscriptionFilterAttributes" />
</class>
+ <!-- BugMute -->
+ <class
+ class=".model.bug.BugMute">
+ <allow
+ interface=".interfaces.bug.IBugMute"/>
+ </class>
+
<!-- BugSubscriptionFilterMute -->
<class
class=".model.bugsubscriptionfilter.BugSubscriptionFilterMute">
=== modified file 'lib/lp/bugs/interfaces/bug.py'
--- lib/lp/bugs/interfaces/bug.py 2011-05-02 16:09:43 +0000
+++ lib/lp/bugs/interfaces/bug.py 2011-05-11 11:47:01 +0000
@@ -14,6 +14,7 @@
'IBugAddForm',
'IBugBecameQuestionEvent',
'IBugDelta',
+ 'IBugMute',
'IBugSet',
'IFileBugData',
'IFrontPageBugAddForm',
@@ -85,6 +86,7 @@
ContentNameField,
Description,
DuplicateBug,
+ PersonChoice,
PublicPersonChoice,
Tag,
Title,
@@ -1210,3 +1212,18 @@
comments = Attribute("Comments to add to the bug.")
attachments = Attribute("Attachments to add to the bug.")
hwdb_submission_keys = Attribute("HWDB submission keys for the bug.")
+
+
+class IBugMute(Interface):
+ """A mute on an IBug."""
+
+ person = PersonChoice(
+ title=_('Person'), required=True, vocabulary='ValidPersonOrTeam',
+ readonly=True, description=_("The person subscribed."))
+ bug = Reference(
+ IBug, title=_("Bug"),
+ required=True, readonly=True,
+ description=_("The bug to be muted."))
+ date_created = Datetime(
+ title=_("The date on which the mute was created."), required=False,
+ readonly=True)
=== modified file 'lib/lp/bugs/interfaces/bugnotification.py'
--- lib/lp/bugs/interfaces/bugnotification.py 2011-04-05 22:34:35 +0000
+++ lib/lp/bugs/interfaces/bugnotification.py 2011-05-11 11:47:01 +0000
@@ -80,15 +80,17 @@
`BugNotificationRecipient` objects.
"""
- def getRecipientFilterData(recipient_to_sources, notifications):
+ def getRecipientFilterData(bug, recipient_to_sources, notifications):
"""Get non-muted recipients mapped to sources & filter descriptions.
+ :param bug:
+ A bug we are collecting filter data for.
:param recipient_to_sources:
A dict of people who are to receive the email to the sources
(BugNotificationRecipients) that represent the subscriptions that
caused the notifications to be sent.
:param notifications: the notifications that are being communicated.
-
+
The dict of recipients may have fewer recipients than were
provided if those users muted all of the subscription filters
that caused them to be sent.
=== modified file 'lib/lp/bugs/model/bug.py'
--- lib/lp/bugs/model/bug.py 2011-05-10 02:23:38 +0000
+++ lib/lp/bugs/model/bug.py 2011-05-11 11:47:01 +0000
@@ -11,6 +11,7 @@
'Bug',
'BugAffectsPerson',
'BugBecameQuestionEvent',
+ 'BugMute',
'BugSet',
'BugTag',
'FileBugData',
@@ -29,6 +30,7 @@
from functools import wraps
from itertools import chain
import operator
+import pytz
import re
from lazr.lifecycle.event import (
@@ -63,6 +65,11 @@
Union,
)
from storm.info import ClassAlias
+from storm.locals import (
+ DateTime,
+ Int,
+ Reference,
+ )
from storm.store import (
EmptyResultSet,
Store,
@@ -138,6 +145,7 @@
from lp.bugs.interfaces.bug import (
IBug,
IBugBecameQuestionEvent,
+ IBugMute,
IBugSet,
IFileBugData,
)
@@ -188,6 +196,7 @@
from lp.registry.interfaces.distroseries import IDistroSeries
from lp.registry.interfaces.person import (
IPersonSet,
+ validate_person,
validate_public_person,
)
from lp.registry.interfaces.product import IProduct
@@ -201,6 +210,7 @@
)
from lp.registry.model.pillar import pillar_sort_key
from lp.registry.model.teammembership import TeamParticipation
+from lp.services.database.stormbase import StormBase
from lp.services.fields import DuplicateBug
from lp.services.propertycache import (
cachedproperty,
@@ -843,40 +853,42 @@
"""See `IBug`."""
return self.personIsSubscribedToDuplicate(person)
+ def _getMutes(self, person):
+ store = Store.of(self)
+ mutes = store.find(
+ BugMute,
+ BugMute.bug == self,
+ BugMute.person == person)
+ return mutes
+
def isMuted(self, person):
"""See `IBug`."""
- store = Store.of(self)
- subscriptions = store.find(
- BugSubscription,
- BugSubscription.bug == self,
- BugSubscription.person == person,
- BugSubscription.bug_notification_level ==
- BugNotificationLevel.NOTHING)
- return not subscriptions.is_empty()
+ mutes = self._getMutes(person)
+ return not mutes.is_empty()
def mute(self, person, muted_by):
"""See `IBug`."""
if person is None:
# This may be a webservice request.
person = muted_by
- # If there's an existing subscription, update it.
- store = Store.of(self)
- subscriptions = store.find(
- BugSubscription,
- BugSubscription.bug == self,
- BugSubscription.person == person)
- if subscriptions.is_empty():
- return self.subscribe(
- person, muted_by, level=BugNotificationLevel.NOTHING)
+ # If it's already muted, ignore the request.
+ mutes = self._getMutes(person)
+ if mutes.is_empty():
+ # XXX: Execute addChange() to add muting to the activity log.
+ BugMute(person, self)
else:
- subscription = subscriptions.one()
- subscription.bug_notification_level = (
- BugNotificationLevel.NOTHING)
- return subscription
+ # It's already muted, pass.
+ pass
def unmute(self, person, unmuted_by):
"""See `IBug`."""
- self.unsubscribe(person, unmuted_by)
+ store = Store.of(self)
+ if person is None:
+ # This may be a webservice request.
+ person = unmuted_by
+ mutes = self._getMutes(person)
+ # XXX: Execute addChange() to add unmuting to the activity log.
+ store.remove(mutes.one())
@property
def subscriptions(self):
@@ -2644,3 +2656,29 @@
def asDict(self):
"""Return the FileBugData instance as a dict."""
return self.__dict__.copy()
+
+
+class BugMute(StormBase):
+ """Contains bugs a person has decided to block notifications from."""
+
+ implements(IBugMute)
+
+ __storm_table__ = "BugMute"
+
+ def __init__(self, person=None, bug=None):
+ if person is not None:
+ self.person = person
+ if bug is not None:
+ self.bug_id = bug.id
+
+ person_id = Int("person", allow_none=False, validator=validate_person)
+ person = Reference(person_id, "Person.id")
+
+ bug_id = Int("bug", allow_none=False)
+ bug = Reference(bug_id, "Bug.id")
+
+ __storm_primary__ = 'person_id', 'bug_id'
+
+ date_created = DateTime(
+ "date_created", allow_none=False, default=UTC_NOW,
+ tzinfo=pytz.UTC)
=== modified file 'lib/lp/bugs/model/bugnotification.py'
--- lib/lp/bugs/model/bugnotification.py 2011-04-05 22:34:35 +0000
+++ lib/lp/bugs/model/bugnotification.py 2011-05-11 11:47:01 +0000
@@ -189,11 +189,17 @@
return bug_notification
- def getRecipientFilterData(self, recipient_to_sources, notifications):
+ def getRecipientFilterData(self, bug, recipient_to_sources, notifications):
"""See `IBugNotificationSet`."""
if not notifications or not recipient_to_sources:
# This is a shortcut that will remove some error conditions.
return {}
+ # Collect bug mute information.
+ from lp.bugs.model.bug import BugMute
+ store = IStore(BugMute)
+ muted_person_ids = set(list(
+ store.find(BugMute.person_id,
+ BugMute.bug == bug)))
# This makes two calls to the database to get all the
# information we need. The first call gets the filter ids and
# descriptions for each recipient, and then we divide up the
@@ -202,6 +208,8 @@
source_person_id_map = {}
recipient_id_map = {}
for recipient, sources in recipient_to_sources.items():
+ if recipient.id in muted_person_ids:
+ continue
source_person_ids = set()
recipient_id_map[recipient.id] = {
'principal': recipient,
@@ -231,14 +239,17 @@
Join(StructuralSubscription,
BugSubscriptionFilter.structural_subscription_id ==
StructuralSubscription.id))
- filter_data = source.find(
- (StructuralSubscription.subscriberID,
- BugSubscriptionFilter.id,
- BugSubscriptionFilter.description),
- In(BugNotificationFilter.bug_notification_id,
- [notification.id for notification in notifications]),
- In(StructuralSubscription.subscriberID,
- source_person_id_map.keys()))
+ if len(source_person_id_map) == 0:
+ filter_data = []
+ else:
+ filter_data = source.find(
+ (StructuralSubscription.subscriberID,
+ BugSubscriptionFilter.id,
+ BugSubscriptionFilter.description),
+ In(BugNotificationFilter.bug_notification_id,
+ [notification.id for notification in notifications]),
+ In(StructuralSubscription.subscriberID,
+ source_person_id_map.keys()))
filter_ids = []
# Record the filters for each source.
for source_person_id, filter_id, filter_description in filter_data:
=== modified file 'lib/lp/bugs/model/bugsubscriptionfilter.py'
--- lib/lp/bugs/model/bugsubscriptionfilter.py 2011-04-05 22:34:35 +0000
+++ lib/lp/bugs/model/bugsubscriptionfilter.py 2011-05-11 11:47:01 +0000
@@ -296,7 +296,7 @@
class BugSubscriptionFilterMute(StormBase):
- """A filter to specialize a *structural* subscription."""
+ """Bug subscription filters a person has decided to block emails from."""
implements(IBugSubscriptionFilterMute)
@@ -312,7 +312,7 @@
person = Reference(person_id, "Person.id")
filter_id = Int("filter", allow_none=False)
- filter = Reference(filter_id, "StructuralSubscription.id")
+ filter = Reference(filter_id, "BugSubscriptionFilter.id")
__storm_primary__ = 'person_id', 'filter_id'
=== modified file 'lib/lp/bugs/scripts/bugnotification.py'
--- lib/lp/bugs/scripts/bugnotification.py 2011-04-05 22:34:35 +0000
+++ lib/lp/bugs/scripts/bugnotification.py 2011-05-11 11:47:01 +0000
@@ -174,7 +174,7 @@
from_address = get_bugmail_from_address(actor, bug)
bug_notification_builder = BugNotificationBuilder(bug, actor)
recipients = getUtility(IBugNotificationSet).getRecipientFilterData(
- recipients, filtered_notifications)
+ bug, recipients, filtered_notifications)
sorted_recipients = sorted(
recipients.items(), key=lambda t: t[0].preferredemail.email)
=== modified file 'lib/lp/bugs/scripts/tests/test_bugnotification.py'
--- lib/lp/bugs/scripts/tests/test_bugnotification.py 2011-05-02 16:53:13 +0000
+++ lib/lp/bugs/scripts/tests/test_bugnotification.py 2011-05-11 11:47:01 +0000
@@ -172,7 +172,7 @@
implements(IBugNotificationSet)
- def getRecipientFilterData(self, recipient_to_sources, notifications):
+ def getRecipientFilterData(self, bug, recipient_to_sources, notifications):
return dict(
(recipient, {'sources': sources, 'filter descriptions': []})
for recipient, sources in recipient_to_sources.items())
=== modified file 'lib/lp/bugs/tests/test_bug.py'
--- lib/lp/bugs/tests/test_bug.py 2011-05-02 16:53:13 +0000
+++ lib/lp/bugs/tests/test_bug.py 2011-05-11 11:47:01 +0000
@@ -8,6 +8,7 @@
from lazr.lifecycle.snapshot import Snapshot
from zope.component import getUtility
from zope.interface import providedBy
+from zope.security.proxy import removeSecurityProxy
from canonical.testing.layers import DatabaseFunctionalLayer
@@ -39,11 +40,9 @@
self.person = self.factory.makePerson()
def test_is_muted_returns_true_for_muted_users(self):
- # Bug.isMuted() will return True if the passed to it has a
- # BugSubscription with a BugNotificationLevel of NOTHING.
+ # Bug.isMuted() will return True if the person passed to it is muted.
with person_logged_in(self.person):
- self.bug.subscribe(
- self.person, self.person, level=BugNotificationLevel.NOTHING)
+ self.bug.mute(self.person, self.person)
self.assertEqual(True, self.bug.isMuted(self.person))
def test_is_muted_returns_false_for_direct_subscribers(self):
@@ -61,14 +60,13 @@
self.assertEqual(False, self.bug.isMuted(self.person))
def test_mute_mutes_user(self):
- # Bug.mute() adds a muted subscription for the user passed to
- # it.
+ # Bug.mute() adds a BugMute record for the person passed to it.
with person_logged_in(self.person):
- muted_subscription = self.bug.mute(
- self.person, self.person)
- self.assertEqual(
- BugNotificationLevel.NOTHING,
- muted_subscription.bug_notification_level)
+ self.bug.mute(self.person, self.person)
+ naked_bug = removeSecurityProxy(self.bug)
+ bug_mute = naked_bug._getMutes(self.person).one()
+ self.assertEqual(self.bug, bug_mute.bug)
+ self.assertEqual(self.person, bug_mute.person)
def test_mute_mutes_muter(self):
# When exposed in the web API, the mute method regards the
@@ -80,14 +78,15 @@
self.assertTrue(self.bug.isMuted(self.person))
def test_mute_mutes_user_with_existing_subscription(self):
- # Bug.mute() will update an existing subscription so that it
- # becomes muted.
+ # Bug.mute() will not touch the existing subscription.
with person_logged_in(self.person):
- subscription = self.bug.subscribe(self.person, self.person)
- muted_subscription = self.bug.mute(self.person, self.person)
- self.assertEqual(subscription, muted_subscription)
+ subscription = self.bug.subscribe(
+ self.person, self.person,
+ level=BugNotificationLevel.METADATA)
+ self.bug.mute(self.person, self.person)
+ self.assertTrue(self.bug.isMuted(self.person))
self.assertEqual(
- BugNotificationLevel.NOTHING,
+ BugNotificationLevel.METADATA,
subscription.bug_notification_level)
def test_unmute_unmutes_user(self):
=== modified file 'lib/lp/bugs/tests/test_bugchanges.py'
--- lib/lp/bugs/tests/test_bugchanges.py 2011-03-23 18:29:09 +0000
+++ lib/lp/bugs/tests/test_bugchanges.py 2011-05-11 11:47:01 +0000
@@ -1763,6 +1763,21 @@
self.assertRecipients(
[self.product_metadata_subscriber, team.teamowner])
+ def test_description_changed_no_muted_email(self):
+ # Users who have muted a bug do not get any bug email for a bug,
+ # even if they are subscribed through a team membership.
+ team = self.factory.makeTeam()
+ team.addMember(self.user, team.teamowner)
+ self.bug.subscribe(team, self.user)
+ self.bug.mute(self.user, self.user)
+
+ old_description = self.changeAttribute(
+ self.bug, 'description', 'New description')
+
+ # self.user is not included among the recipients.
+ self.assertRecipients(
+ [self.product_metadata_subscriber, team.teamowner])
+
def test_no_lifecycle_email_despite_structural_subscription(self):
# If a person has a structural METADATA subscription,
# and a direct LIFECYCLE subscription, they should
=== modified file 'lib/lp/bugs/tests/test_bugnotification.py'
--- lib/lp/bugs/tests/test_bugnotification.py 2011-05-03 04:34:35 +0000
+++ lib/lp/bugs/tests/test_bugnotification.py 2011-05-11 11:47:01 +0000
@@ -233,11 +233,11 @@
def test_getRecipientFilterData_empty(self):
# When there is empty input, there is empty output.
self.assertEqual(
- BugNotificationSet().getRecipientFilterData({}, []),
+ BugNotificationSet().getRecipientFilterData(self.bug, {}, []),
{})
self.assertEqual(
BugNotificationSet().getRecipientFilterData(
- {}, [self.notification]),
+ self.bug, {}, [self.notification]),
{})
def test_getRecipientFilterData_other_persons(self):
@@ -259,7 +259,7 @@
subscriber2: {'sources': sources2,
'filter descriptions': [u'Special Filter!']}},
BugNotificationSet().getRecipientFilterData(
- {self.subscriber: sources, subscriber2: sources2},
+ self.bug, {self.subscriber: sources, subscriber2: sources2},
[self.notification, notification2]))
def test_getRecipientFilterData_match(self):
@@ -271,7 +271,7 @@
{self.subscriber: {'sources': sources,
'filter descriptions': ['Special Filter!']}},
BugNotificationSet().getRecipientFilterData(
- {self.subscriber: sources}, [self.notification]))
+ self.bug, {self.subscriber: sources}, [self.notification]))
def test_getRecipientFilterData_multiple_notifications_match(self):
# When there are bug filters for the recipient for multiple
@@ -285,7 +285,7 @@
{self.subscriber: {'sources': sources,
'filter descriptions': ['Another Filter!', 'Special Filter!']}},
BugNotificationSet().getRecipientFilterData(
- {self.subscriber: sources},
+ self.bug, {self.subscriber: sources},
[self.notification, self.notification2]))
def test_getRecipientFilterData_mute(self):
@@ -300,7 +300,7 @@
self.assertEqual(
{},
BugNotificationSet().getRecipientFilterData(
- {self.subscriber: sources}, [self.notification]))
+ self.bug, {self.subscriber: sources}, [self.notification]))
def test_getRecipientFilterData_mute_one_person_of_two(self):
self.includeFilterInNotification()
@@ -321,7 +321,7 @@
{subscriber2: {'sources': sources2,
'filter descriptions': [u'Special Filter!']}},
BugNotificationSet().getRecipientFilterData(
- {self.subscriber: sources, subscriber2: sources2},
+ self.bug, {self.subscriber: sources, subscriber2: sources2},
[self.notification, notification2]))
def test_getRecipientFilterData_mute_one_filter_of_two(self):
@@ -337,7 +337,7 @@
{self.subscriber: {'sources': sources,
'filter descriptions': ['Another Filter!']}},
BugNotificationSet().getRecipientFilterData(
- {self.subscriber: sources},
+ self.bug, {self.subscriber: sources},
[self.notification, self.notification2]))
def test_getRecipientFilterData_mute_both_filters_mutes(self):
@@ -356,9 +356,51 @@
self.assertEqual(
{},
BugNotificationSet().getRecipientFilterData(
- {self.subscriber: sources},
+ self.bug, {self.subscriber: sources},
[self.notification, self.notification2]))
+ def test_getRecipientFilterData_mute_bug_mutes(self):
+ # Mute the bug for the subscriber.
+ self.team = self.factory.makeTeam()
+ self.subscriber.join(self.team)
+
+ self.bug.mute(self.subscriber, self.subscriber)
+ sources = list(self.notification.recipients)
+ # Perform the test.
+ self.assertEqual(
+ {},
+ BugNotificationSet().getRecipientFilterData(
+ self.bug, {self.subscriber: sources}, [self.notification]))
+
+ def test_getRecipientFilterData_mute_bug_mutes_only_themselves(self):
+ # Mute the bug for the subscriber.
+ self.bug.mute(self.subscriber, self.subscriber)
+
+ # Notification for the other person still goes through.
+ person = self.factory.makePerson(name='other')
+ self.addNotificationRecipient(self.notification, person)
+
+ sources = list(self.notification.recipients)
+
+ # Perform the test.
+ self.assertEqual(
+ { person: { 'filter descriptions': [],
+ 'sources': sources } },
+ BugNotificationSet().getRecipientFilterData(
+ self.bug, {self.subscriber: sources,
+ person: sources},
+ [self.notification]))
+
+ def test_getRecipientFilterData_mute_bug_mutes_filter(self):
+ # Mute the bug for the subscriber.
+ self.bug.mute(self.subscriber, self.subscriber)
+ self.includeFilterInNotification(description=u'Special Filter!')
+ sources = list(self.notification.recipients)
+ self.assertEqual(
+ {},
+ BugNotificationSet().getRecipientFilterData(
+ self.bug, {self.subscriber: sources}, [self.notification]))
+
class TestNotificationProcessingWithoutRecipients(TestCaseWithFactory):
"""Adding notificatons without any recipients does not cause any harm.
=== modified file 'lib/lp/registry/model/person.py'
--- lib/lp/registry/model/person.py 2011-05-10 18:02:28 +0000
+++ lib/lp/registry/model/person.py 2011-05-11 11:47:01 +0000
@@ -4663,7 +4663,7 @@
if person.preferredemail:
return [person]
elif person.is_team:
- # Get transitive members of team with a preferred email.
+ # Get transitive members of team without a preferred email.
return _get_recipients_for_team(person)
else:
return []
Follow ups
