launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #02466
[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.
== Proposed fix ==
== Pre-implementation notes ==
== Implementation details ==
== Tests ==
== Demo and Q/A ==
= Launchpad lint =
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/bugs/adapters/tests/__init__.py
lib/lp/bugs/adapters/bugchange.py
lib/lp/bugs/adapters/tests/test_bugchange.py
lib/lp/bugs/interfaces/bugchange.py
lib/lp/bugs/interfaces/bugtask.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-01 16:05:39 +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-01 16:05:39 +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-01 16:05:39 +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 2010-12-17 04:26:11 +0000
+++ lib/lp/bugs/interfaces/bugtask.py 2011-02-01 16:05:39 +0000
@@ -350,13 +350,6 @@
BUGS_WITHOUT_BRANCHES = Item("Show only Bugs without linked Branches")
-# XXX: Brad Bollenbach 2005-12-02 bugs=5320:
-# In theory, INCOMPLETE belongs in UNRESOLVED_BUGTASK_STATUSES, but the
-# semantics of our current reports would break if it were added to the
-# list below.
-
-# XXX: matsubara 2006-02-02 bug=4201:
-# I added the INCOMPLETE as a short-term solution.
UNRESOLVED_BUGTASK_STATUSES = (
BugTaskStatus.NEW,
BugTaskStatus.INCOMPLETE,
Follow ups
