← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~danilo/launchpad/lifecycle-subscription into lp:launchpad

 

Данило Шеган has proposed merging lp:~danilo/launchpad/lifecycle-subscription into lp:launchpad with lp:~danilo/launchpad/remove-bugtaskadded as a prerequisite.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~danilo/launchpad/lifecycle-subscription/+merge/48185

= Implement BugNotificationLevel.LIFECYCLE for direct subscriptions =

With bug subscription work, one of the "levels" at which one can subscribe to a bug is "LIFECYCLE": only when bug is closed and/or reopened should one get an email about it.

This implements a 'change_level' for every IBugChange (examples: BugTaskStatusChange or BugDuplicateChange) which evaluates to LIFECYCLE when appropriate, and to METADATA otherwise.

== Pre-implementation notes ==

Discussed this with Graham at length (mostly about what type of change should be a lifecycle change), and add_bug_change_notifications specifically with Gavin.  Unfortunately, there're no unit tests for add_bug_change_notifications, so I strived to do minimal changes there, even though every look at it made me want to rewrite it altogether.

== Implementation details ==

First of all, test in lib/lp/bugs/adapters/tests/test_bugchange.py was not even executing before (missing __init__.py for the module), so I fixed that. I've also moved the test to the more recent format (using TestCaseWithFactory and similar).  The test there for change_level is very thorough, except that it doesn't cover *all* the cases.  The most reasonable way to cover all the cases is to eg. go over all RESOLVED_BUGTASK_STATUSES and UNRESOLVED_BUGTASK_STATUSES and generate test cases for each (to simulate going from RESOLVED to UNRESOLVED status, or reopening a bug, or the other way around—closing a bug by going from UNRESOLVED to RESOLVED), however, I always feel as if I am cheating when my tests resemble actual code too much (i.e. testing it this way and then actually using RESOLVED/UNRESOLVED_BUGTASK_STATUSES in the code asserts that we are indeed using that particular variable, but doesn't tell us if that makes any sense—if we ever stop using this variable in the rest of bugs code, such test would keep on passing even though it doesn't reflect reality; granted, that's what unit testing should be, so I'd be happy to change that and provide "comprehensive" testing if a reviewer wants me to).

The implementation of change_level is quite simple, except for status transitions and for duplicate changes.  Some of it is by no means perfect (eg. in duplicate checks, we only check default_bugtask for a bug this is marked to be duplicate of, but coming up with 'correct' behaviour for more than one bug task is very hard, if not impossible).  The INCOMPLETE status is special cased in status transitions for one particular reason: even if you are subscribed only at the LIFECYCLE level, if you were the one who filed a bug, then you should still get the email when bug is marked as INCOMPLETE, because it depends on *your* input.

For add_bug_changes_notification, I add a basic unit test to confirm everything works, and then extend it to support LIFECYCLE subscribers.  We still need to get METADATA subscribers, because they still get all the emails, but for every individual change, we might add LIFECYCLE subscribers as well.  I believe the side effect of this is that LIFECYCLE subscribers would get more details in that single email, but wouldn't get more than one email.

== Tests ==

bin/test -cvvt BugChangeLevel -t add_bug_change_notifications

== Demo and Q/A ==

I'll still have to figure out how to best QA this, but I am sure gmb/allenap will be happy to help out. :)

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/bugs/adapters/bugchange.py
  lib/lp/bugs/adapters/tests/__init__.py
  lib/lp/bugs/adapters/tests/test_bugchange.py
  lib/lp/bugs/interfaces/bugchange.py
  lib/lp/bugs/interfaces/bugtask.py
  lib/lp/bugs/model/bug.py
  lib/lp/bugs/subscribers/bug.py
  lib/lp/bugs/subscribers/tests/
  lib/lp/bugs/subscribers/tests/__init__.py
  lib/lp/bugs/subscribers/tests/test_bug.py

-- 
https://code.launchpad.net/~danilo/launchpad/lifecycle-subscription/+merge/48185
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~danilo/launchpad/lifecycle-subscription into lp:launchpad.
=== modified file 'lib/lp/bugs/adapters/bugchange.py'
--- lib/lp/bugs/adapters/bugchange.py	2010-09-02 10:29:37 +0000
+++ lib/lp/bugs/adapters/bugchange.py	2011-02-02 11:21:14 +0000
@@ -38,8 +38,14 @@
 from canonical.launchpad.browser.librarian import ProxiedLibraryFileAlias
 from canonical.launchpad.webapp.publisher import canonical_url
 from lp.bugs.interfaces.bugchange import IBugChange
-from lp.bugs.interfaces.bugtask import IBugTask
+from lp.bugs.interfaces.bugtask import (
+    BugTaskStatus,
+    IBugTask,
+    RESOLVED_BUGTASK_STATUSES,
+    UNRESOLVED_BUGTASK_STATUSES,
+    )
 from lp.registry.interfaces.product import IProduct
+from lp.registry.enum import BugNotificationLevel
 
 
 class NoBugChangeFoundError(Exception):
@@ -110,6 +116,9 @@
 
     implements(IBugChange)
 
+    # Most changes will be at METADATA level.
+    change_level = BugNotificationLevel.METADATA
+
     def __init__(self, when, person):
         self.person = person
         self.when = when
@@ -341,9 +350,50 @@
         return {'text': notification_text}
 
 
+def _is_status_change_lifecycle_change(old_status, new_status):
+    """Is a status change a lifecycle change."""
+    # Bug is moving from one of unresolved bug statuses (like
+    # 'in progress') to one of resolved ('fix released').
+    bug_is_closed = (old_status in UNRESOLVED_BUGTASK_STATUSES and
+                     new_status in RESOLVED_BUGTASK_STATUSES)
+
+    # Bug is moving back from one of resolved bug statuses (reopening).
+    bug_is_reopened = (old_status in RESOLVED_BUGTASK_STATUSES and
+                       new_status in UNRESOLVED_BUGTASK_STATUSES)
+    return bug_is_closed or bug_is_reopened
+
+
 class BugDuplicateChange(AttributeChange):
     """Describes a change to a bug's duplicate marker."""
 
+    @property
+    def change_level(self):
+        lifecycle = False
+        old_bug = self.old_value
+        new_bug = self.new_value
+        if old_bug is not None and new_bug is not None:
+            # Bug was already a duplicate of one bug,
+            # and we are changing it to be a duplicate of another bug.
+            lifecycle = _is_status_change_lifecycle_change(
+                old_bug.default_bugtask.status,
+                new_bug.default_bugtask.status)
+        elif new_bug is not None:
+            # old_bug is None here, so we are just adding a duplicate marker.
+            lifecycle = (new_bug.default_bugtask.status in
+                         RESOLVED_BUGTASK_STATUSES)
+        elif old_bug is not None:
+            # Unmarking a bug as duplicate.  This is lifecycle change
+            # only if bug has been reopened as a result.
+            lifecycle = (old_bug.default_bugtask.status in
+                         RESOLVED_BUGTASK_STATUSES)
+        else:
+            pass
+
+        if lifecycle:
+            return BugNotificationLevel.LIFECYCLE
+        else:
+            return BugNotificationLevel.METADATA
+
     def getBugActivity(self):
         if self.old_value is not None and self.new_value is not None:
             return {
@@ -722,6 +772,24 @@
     # Use `status.title` in activity records and notifications.
     display_attribute = 'title'
 
+    @property
+    def change_level(self):
+        """See `IBugChange`."""
+        # Is bug being closed or reopened.
+        lifecycle_change = _is_status_change_lifecycle_change(
+            self.old_value, self.new_value)
+
+        # When bug task is put into INCOMPLETE status, and subscriber
+        # is the person who originally reported the bug, we still notify
+        # them because bug now depends on their input.
+        subscriber_is_asked_for_info = (
+            self.new_value == BugTaskStatus.INCOMPLETE and
+            self.bug_task.bug.owner == self.person)
+
+        if (lifecycle_change or subscriber_is_asked_for_info):
+            return BugNotificationLevel.LIFECYCLE
+        return BugNotificationLevel.METADATA
+
 
 class BugTaskMilestoneChange(BugTaskAttributeChange):
     """Represents a change in BugTask.milestone."""
@@ -752,6 +820,7 @@
 
     def getBugActivity(self):
         """See `IBugChange`."""
+
         def assignee_for_display(assignee):
             if assignee is None:
                 return None
@@ -766,6 +835,7 @@
 
     def getBugNotification(self):
         """See `IBugChange`."""
+
         def assignee_for_display(assignee):
             if assignee is None:
                 return "(unassigned)"

=== added file 'lib/lp/bugs/adapters/tests/__init__.py'
=== modified file 'lib/lp/bugs/adapters/tests/test_bugchange.py'
--- lib/lp/bugs/adapters/tests/test_bugchange.py	2010-10-04 19:50:45 +0000
+++ lib/lp/bugs/adapters/tests/test_bugchange.py	2011-02-02 11:21:14 +0000
@@ -5,31 +5,27 @@
 
 __metaclass__ = type
 
-import unittest
-
-from canonical.launchpad.ftests import (
-    ANONYMOUS,
-    login,
-    logout,
-    )
-from canonical.testing.layers import LaunchpadFunctionalLayer
+from canonical.launchpad.webapp.publisher import canonical_url
+from canonical.testing.layers import ZopelessDatabaseLayer
 from lp.bugs.adapters.bugchange import (
     BUG_CHANGE_LOOKUP,
+    BugDescriptionChange,
     get_bug_change_class,
+    get_bug_changes,
     )
-from lp.testing.factory import LaunchpadObjectFactory
-
-
-class BugChangeTestCase(unittest.TestCase):
-
-    layer = LaunchpadFunctionalLayer
+from lp.bugs.adapters.bugdelta import BugDelta
+from lp.bugs.interfaces.bugtask import BugTaskStatus
+from lp.bugs.model.bugtask import BugTaskDelta
+from lp.registry.enum import BugNotificationLevel
+from lp.testing import TestCaseWithFactory
+
+
+class BugChangeTestCase(TestCaseWithFactory):
+
+    layer = ZopelessDatabaseLayer
 
     def setUp(self):
-        login(ANONYMOUS)
-        self.factory = LaunchpadObjectFactory()
-
-    def tearDown(self):
-        logout()
+        super(BugChangeTestCase, self).setUp()
 
     def test_get_bug_change_class(self):
         # get_bug_change_class() should return whatever is contained
@@ -45,5 +41,182 @@
                 "Got %s." % (expected, field_name, received))
 
 
-def test_suite():
-    return unittest.TestLoader().loadTestsFromName(__name__)
+class BugChangeLevelTestCase(TestCaseWithFactory):
+
+    layer = ZopelessDatabaseLayer
+
+    def setUp(self):
+        super(BugChangeLevelTestCase, self).setUp()
+        self.bug = self.factory.makeBug()
+        self.bugtask = self.bug.default_bugtask
+        self.user = self.factory.makePerson()
+
+    def createDelta(self, user=None, **kwargs):
+        if user is None:
+            user = self.user
+        return BugDelta(
+            bug=self.bug,
+            bugurl=canonical_url(self.bug),
+            user=user,
+            **kwargs)
+
+    def test_change_level_metadata_description(self):
+        # Changing a bug description is considered to have change_level
+        # of BugNotificationLevel.METADATA.
+        bug_delta = self.createDelta(
+            description={'new': 'new description',
+                         'old': self.bug.description})
+
+        change = list(get_bug_changes(bug_delta))[0]
+        self.assertTrue(isinstance(change, BugDescriptionChange))
+        self.assertEquals(BugNotificationLevel.METADATA,
+                          change.change_level)
+
+    def test_change_level_lifecycle_status_closing(self):
+        # Changing a bug task status from NEW to FIXRELEASED makes this
+        # change a BugNotificationLevel.LIFECYCLE change.
+        bugtask_delta = BugTaskDelta(
+            bugtask=self.bugtask,
+            status={'old': BugTaskStatus.NEW,
+                    'new': BugTaskStatus.FIXRELEASED})
+        bug_delta = self.createDelta(
+            bugtask_deltas=bugtask_delta)
+
+        change = list(get_bug_changes(bug_delta))[0]
+        self.assertEquals(BugNotificationLevel.LIFECYCLE,
+                          change.change_level)
+
+    def test_change_level_lifecycle_status_reopening(self):
+        # Changing a bug task status from FIXRELEASED to TRIAGED makes this
+        # change a BugNotificationLevel.LIFECYCLE change.
+        bugtask_delta = BugTaskDelta(
+            bugtask=self.bugtask,
+            status={'old': BugTaskStatus.FIXRELEASED,
+                    'new': BugTaskStatus.TRIAGED})
+        bug_delta = self.createDelta(
+            bugtask_deltas=bugtask_delta)
+
+        change = list(get_bug_changes(bug_delta))[0]
+        self.assertEquals(BugNotificationLevel.LIFECYCLE,
+                          change.change_level)
+
+    def test_change_level_metadata_status_worked_on(self):
+        # Changing a bug task status from TRIAGED to FIXCOMMITTED makes this
+        # change a BugNotificationLevel.METADATA change.
+        bugtask_delta = BugTaskDelta(
+            bugtask=self.bugtask,
+            status={'old': BugTaskStatus.TRIAGED,
+                    'new': BugTaskStatus.FIXCOMMITTED})
+        bug_delta = self.createDelta(
+            bugtask_deltas=bugtask_delta)
+
+        change = list(get_bug_changes(bug_delta))[0]
+        self.assertEquals(BugNotificationLevel.METADATA,
+                          change.change_level)
+
+    def test_change_level_metadata_status_stays_closed(self):
+        # Changing a bug task status from OPINION to WONTFIX makes this
+        # change a BugNotificationLevel.METADATA change.
+        bugtask_delta = BugTaskDelta(
+            bugtask=self.bugtask,
+            status={'old': BugTaskStatus.OPINION,
+                    'new': BugTaskStatus.WONTFIX})
+        bug_delta = self.createDelta(
+            bugtask_deltas=bugtask_delta)
+
+        change = list(get_bug_changes(bug_delta))[0]
+        self.assertEquals(BugNotificationLevel.METADATA,
+                          change.change_level)
+
+    def test_change_level_metadata_status_incomplete_for_someone_else(self):
+        # Changing a bug task status from NEW to INCOMPLETE makes this
+        # change a BugNotificationLevel.METADATA change if subscriber is
+        # not the original reporter of the bug as well.
+        bugtask_delta = BugTaskDelta(
+            bugtask=self.bugtask,
+            status={'old': BugTaskStatus.NEW,
+                    'new': BugTaskStatus.INCOMPLETE})
+        bug_delta = self.createDelta(
+            bugtask_deltas=bugtask_delta)
+
+        change = list(get_bug_changes(bug_delta))[0]
+        self.assertEquals(BugNotificationLevel.METADATA,
+                          change.change_level)
+
+    def test_change_level_lifecycle_status_incomplete_for_subscriber(self):
+        # Changing a bug task status from NEW to INCOMPLETE makes this
+        # change a BugNotificationLevel.LIFECYCLE change if subscriber is
+        # the original reporter of the bug as well: this ensures that
+        # bug reporter gets a notification about their input needed even
+        # if they are only subscribed on the LIFECYCLE level.
+        bugtask_delta = BugTaskDelta(
+            bugtask=self.bugtask,
+            status={'old': BugTaskStatus.NEW,
+                    'new': BugTaskStatus.INCOMPLETE})
+        bug_delta = self.createDelta(
+            user=self.bug.owner,
+            bugtask_deltas=bugtask_delta)
+
+        change = list(get_bug_changes(bug_delta))[0]
+        self.assertEquals(BugNotificationLevel.LIFECYCLE,
+                          change.change_level)
+
+    def test_change_level_metadata_duplicate_of_unresolved(self):
+        # Marking a bug as a duplicate of an unresolved bug is a
+        # simple BugNotificationLevel.METADATA change.
+        duplicate_of = self.factory.makeBug()
+        duplicate_of.default_bugtask.transitionToStatus(
+            BugTaskStatus.NEW, self.user)
+        bug_delta = self.createDelta(
+            user=self.bug.owner,
+            duplicateof={'old': None,
+                         'new': duplicate_of})
+
+        change = list(get_bug_changes(bug_delta))[0]
+        self.assertEquals(BugNotificationLevel.METADATA,
+                          change.change_level)
+
+    def test_change_level_lifecycle_duplicate_of_resolved(self):
+        # Marking a bug as a duplicate of a resolved bug is
+        # a BugNotificationLevel.LIFECYCLE change.
+        duplicate_of = self.factory.makeBug()
+        duplicate_of.default_bugtask.transitionToStatus(
+            BugTaskStatus.FIXRELEASED, self.user)
+        bug_delta = self.createDelta(
+            user=self.bug.owner,
+            duplicateof={'old': None,
+                         'new': duplicate_of})
+
+        change = list(get_bug_changes(bug_delta))[0]
+        self.assertEquals(BugNotificationLevel.LIFECYCLE,
+                          change.change_level)
+
+    def test_change_level_metadata_not_duplicate_of_unresolved(self):
+        # Un-marking a bug as a duplicate of an unresolved bug is a
+        # simple BugNotificationLevel.METADATA change.
+        duplicate_of = self.factory.makeBug()
+        duplicate_of.default_bugtask.transitionToStatus(
+            BugTaskStatus.NEW, self.user)
+        bug_delta = self.createDelta(
+            user=self.bug.owner,
+            duplicateof={'new': None,
+                         'old': duplicate_of})
+
+        change = list(get_bug_changes(bug_delta))[0]
+        self.assertEquals(BugNotificationLevel.METADATA,
+                          change.change_level)
+
+    def test_change_level_lifecycle_not_duplicate_of_resolved(self):
+        # Un-marking a bug as a duplicate of a resolved bug is
+        # a BugNotificationLevel.LIFECYCLE change.
+        duplicate_of = self.factory.makeBug()
+        duplicate_of.default_bugtask.transitionToStatus(
+            BugTaskStatus.FIXRELEASED, self.user)
+        bug_delta = self.createDelta(
+            user=self.bug.owner,
+            duplicateof={'new': None,
+                         'old': duplicate_of})
+
+        change = list(get_bug_changes(bug_delta))[0]
+        self.assertEquals(BugNotificationLevel.LIFECYCLE,
+                          change.change_level)

=== modified file 'lib/lp/bugs/interfaces/bugchange.py'
--- lib/lp/bugs/interfaces/bugchange.py	2010-08-20 20:31:18 +0000
+++ lib/lp/bugs/interfaces/bugchange.py	2011-02-02 11:21:14 +0000
@@ -21,6 +21,8 @@
 
     when = Attribute("The timestamp for the BugChange.")
     person = Attribute("The Person who made the change.")
+    change_level = Attribute(
+        "Type of the change to a bug expressed as `BugNotificationLevel`.")
 
     def getBugActivity():
         """Return the `BugActivity` data for this change as a dict."""

=== modified file 'lib/lp/bugs/interfaces/bugtask.py'
--- lib/lp/bugs/interfaces/bugtask.py	2011-01-28 21:02:14 +0000
+++ lib/lp/bugs/interfaces/bugtask.py	2011-02-02 11:21:14 +0000
@@ -352,6 +352,7 @@
     BUGS_WITHOUT_BRANCHES = Item("Show only Bugs without linked Branches")
 
 
+<<<<<<< TREE
 class BugBlueprintSearch(EnumeratedType):
     """Bug blueprint search option.
 
@@ -373,6 +374,8 @@
 
 # XXX: matsubara 2006-02-02 bug=4201:
 # I added the INCOMPLETE as a short-term solution.
+=======
+>>>>>>> MERGE-SOURCE
 UNRESOLVED_BUGTASK_STATUSES = (
     BugTaskStatus.NEW,
     BugTaskStatus.INCOMPLETE,

=== modified file 'lib/lp/bugs/model/bug.py'
--- lib/lp/bugs/model/bug.py	2011-01-31 09:24:13 +0000
+++ lib/lp/bugs/model/bug.py	2011-02-02 11:21:14 +0000
@@ -436,7 +436,7 @@
         """See `IBug`."""
         indexed_messages = self._indexed_messages(include_bugmessage=True)
         for indexed_message, bugmessage in indexed_messages:
-            if bugmessage.index != indexed_message.index: 
+            if bugmessage.index != indexed_message.index:
                 bugmessage.index = indexed_message.index
 
     @property
@@ -1018,7 +1018,7 @@
                                      include_master_dupe_subscribers=False):
         """See `IBug`."""
         recipients = BugNotificationRecipients(duplicateof=duplicateof)
-        self.getDirectSubscribers(recipients)
+        self.getDirectSubscribers(recipients, level=level)
         if self.private:
             assert self.getIndirectSubscribers() == [], (
                 "Indirect subscribers found on private bug. "

=== modified file 'lib/lp/bugs/subscribers/bug.py'
--- lib/lp/bugs/subscribers/bug.py	2010-12-24 11:02:19 +0000
+++ lib/lp/bugs/subscribers/bug.py	2011-02-02 11:21:14 +0000
@@ -219,7 +219,7 @@
                 no_dupe_master_recipients = (
                     bug_delta.bug.getBugNotificationRecipients(
                         old_bug=bug_delta.bug_before_modification,
-                        level=BugNotificationLevel.METADATA,
+                        level=change.change_level,
                         include_master_dupe_subscribers=False))
                 bug_delta.bug.addChange(
                     change, recipients=no_dupe_master_recipients)
@@ -231,6 +231,13 @@
                         recipients.remove(person)
                 bug_delta.bug.addChange(change, recipients=recipients)
             else:
+                if change.change_level == BugNotificationLevel.LIFECYCLE:
+                    change_recipients = (
+                        bug_delta.bug.getBugNotificationRecipients(
+                            old_bug=bug_delta.bug_before_modification,
+                            level=change.change_level,
+                            include_master_dupe_subscribers=False))
+                    recipients.update(change_recipients)
                 bug_delta.bug.addChange(change, recipients=recipients)
         else:
             bug_delta.bug.addChangeNotification(

=== added directory 'lib/lp/bugs/subscribers/tests'
=== added file 'lib/lp/bugs/subscribers/tests/__init__.py'
=== added file 'lib/lp/bugs/subscribers/tests/test_bug.py'
--- lib/lp/bugs/subscribers/tests/test_bug.py	1970-01-01 00:00:00 +0000
+++ lib/lp/bugs/subscribers/tests/test_bug.py	2011-02-02 11:21:14 +0000
@@ -0,0 +1,125 @@
+# Copyright 2011 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+from storm.store import Store
+
+from canonical.launchpad.webapp.publisher import canonical_url
+from canonical.testing.layers import ZopelessDatabaseLayer
+from lp.bugs.adapters.bugdelta import BugDelta
+from lp.bugs.interfaces.bugtask import BugTaskStatus
+from lp.bugs.model.bugtask import BugTaskDelta
+from lp.bugs.model.bugnotification import (
+    BugNotification,
+    BugNotificationRecipient,
+    )
+from lp.bugs.subscribers.bug import add_bug_change_notifications
+from lp.registry.enum import BugNotificationLevel
+from lp.registry.model.person import Person
+from lp.testing import TestCaseWithFactory
+
+
+class BugSubscriberTestCase(TestCaseWithFactory):
+
+    layer = ZopelessDatabaseLayer
+
+    def setUp(self):
+        super(BugSubscriberTestCase, self).setUp()
+        self.bug = self.factory.makeBug()
+        self.bugtask = self.bug.default_bugtask
+        self.user = self.factory.makePerson()
+        self.lifecycle_subscriber = self.newSubscriber(
+            self.bug, 'lifecycle-subscriber', BugNotificationLevel.LIFECYCLE)
+        self.metadata_subscriber = self.newSubscriber(
+            self.bug, 'metadata-subscriber', BugNotificationLevel.METADATA)
+        self.old_persons = set(self.getNotifiedPersons(include_all=True))
+
+    def createDelta(self, user=None, **kwargs):
+        if user is None:
+            user = self.user
+        return BugDelta(
+            bug=self.bug,
+            bugurl=canonical_url(self.bug),
+            user=user,
+            **kwargs)
+
+    def newSubscriber(self, bug, name, level):
+        # Create a new bug subscription with a new person.
+        subscriber = self.factory.makePerson(name=name)
+        subscription = bug.subscribe(subscriber, subscriber,
+                                     level=level)
+        return subscriber
+
+    def getNotifiedPersons(self, include_all=False):
+        notified_persons = Store.of(self.bug).find(
+            Person,
+            BugNotification.id==BugNotificationRecipient.bug_notificationID,
+            BugNotificationRecipient.personID==Person.id,
+            BugNotification.bugID==self.bug.id)
+        if include_all:
+            return list(notified_persons)
+        else:
+            return set(notified_persons) - self.old_persons
+
+    def test_add_bug_change_notifications_metadata(self):
+        # Changing a bug description is considered to have change_level
+        # of BugNotificationLevel.METADATA.
+        bug_delta = self.createDelta(
+            description={'new': 'new description',
+                         'old': self.bug.description})
+
+        add_bug_change_notifications(bug_delta)
+
+        self.assertContentEqual([self.metadata_subscriber],
+                                self.getNotifiedPersons())
+
+    def test_add_bug_change_notifications_lifecycle(self):
+        # Changing a bug description is considered to have change_level
+        # of BugNotificationLevel.LIFECYCLE.
+        bugtask_delta = BugTaskDelta(
+            bugtask=self.bugtask,
+            status={'old': BugTaskStatus.NEW,
+                    'new': BugTaskStatus.FIXRELEASED})
+        bug_delta = self.createDelta(
+            bugtask_deltas=bugtask_delta)
+
+        add_bug_change_notifications(bug_delta)
+
+        # Both a LIFECYCLE and METADATA subscribers get notified.
+        self.assertContentEqual([self.metadata_subscriber,
+                                 self.lifecycle_subscriber],
+                                self.getNotifiedPersons())
+
+    def test_add_bug_change_notifications_duplicate_lifecycle(self):
+        # Marking a bug as a duplicate of a resolved bug is
+        # a lifecycle change.
+        duplicate_of = self.factory.makeBug()
+        duplicate_of.default_bugtask.transitionToStatus(
+            BugTaskStatus.FIXRELEASED, self.user)
+        bug_delta = self.createDelta(
+            user=self.bug.owner,
+            duplicateof={'old': None,
+                         'new': duplicate_of})
+
+        add_bug_change_notifications(bug_delta)
+
+        # Both a LIFECYCLE and METADATA subscribers get notified.
+        self.assertContentEqual([self.metadata_subscriber,
+                                 self.lifecycle_subscriber],
+                                self.getNotifiedPersons())
+
+    def test_add_bug_change_notifications_duplicate_metadata(self):
+        # Marking a bug as a duplicate of a unresolved bug is
+        # a lifecycle change.
+        duplicate_of = self.factory.makeBug()
+        duplicate_of.default_bugtask.transitionToStatus(
+            BugTaskStatus.INPROGRESS, self.user)
+        bug_delta = self.createDelta(
+            user=self.bug.owner,
+            duplicateof={'old': None,
+                         'new': duplicate_of})
+
+        add_bug_change_notifications(bug_delta)
+
+        # Only METADATA subscribers get notified.
+        self.assertContentEqual([self.metadata_subscriber],
+                                self.getNotifiedPersons())