Merge lp:~wallyworld/launchpad/delete-bugtask-log-activity-1324 into lp:launchpad

[Ian Booth]

[r=abentley,sinzui][bug=1342] Record bug activity and send email when bug task is deleted.

Record bug activity and send email when bug task is deleted.

== Implementation ==

Provide a new bug change class: BugTaskDeleted
Provide an event subscriber for bugtask delete events. Record the bug change activity in the subscriber.

== Tests ==

Add a new test to TestBugChanges:
  - test_bugtask_deleted

Add a new test case similar to test_bug_task_modification.TestModificationNotification"
  - test_bug_task_deletion.TestModificationNotification
(check that email is generated and correct header is present)

== Lint ==

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/bugs/configure.zcml
  lib/lp/bugs/adapters/bugchange.py
  lib/lp/bugs/mail/tests/test_bug_task_deletion.py
  lib/lp/bugs/model/bugtask.py
  lib/lp/bugs/subscribers/bugactivit

[Aaron Bentley (abentley)]

Why the @block_implicit_flushes ?
If you plan on landing this branch separately from lp:~wallyworld/launchpad/delete-bugtasks-1324, I recommend creating a separate bug for it. It was a bit surprising that the linked bug was "Can't delete spurious "Affects" lines (bugtasks) from bug reports"

review: Approve

[Curtis Hovey (sinzui)]

This looks good to land after you fix the copyright date in test_bug_task_deletion.py.

We need to note that in when we create private projects, the bugtask.target.name may not be readable. Since this is serialised data, we will probably use <hidden name>.

review: Approve (code)

[Ian Booth (wallyworld)]

> Why the @block_implicit_flushes ?

I copied the existing notify_bugtask_added() method which used this.

> If you plan on landing this branch separately from lp:~wallyworld/launchpad
> /delete-bugtasks-1324, I recommend creating a separate bug for it. It was a
> bit surprising that the linked bug was "Can't delete spurious "Affects" lines
> (bugtasks) from bug reports"

This branch is part 2 of the work to fix that bug. The first branch did the core work, this one adds the activity recording. I am going to land both together.

=== modified file 'lib/lp/bugs/adapters/bugchange.py'
--- lib/lp/bugs/adapters/bugchange.py	2011-09-23 12:26:15 +0000
+++ lib/lp/bugs/adapters/bugchange.py	2011-10-19 23:52:28 +0000
@@ -27,6 +27,7 @@
     'BugTaskAdded',
     'BugTaskAssigneeChange',
     'BugTaskBugWatchChange',
+    'BugTaskDeleted',
     'BugTaskImportanceChange',
     'BugTaskMilestoneChange',
     'BugTaskStatusChange',
@@ -260,6 +261,27 @@
             }
 
 
+class BugTaskDeleted(BugChangeBase):
+    """A bugtask was removed from the bug."""
+
+    def __init__(self, when, person, bugtask):
+        super(BugTaskDeleted, self).__init__(when, person)
+        self.targetname = bugtask.bugtargetname
+
+    def getBugActivity(self):
+        """See `IBugChange`."""
+        return dict(
+            whatchanged='bug task deleted',
+            oldvalue=self.targetname)
+
+    def getBugNotification(self):
+        """See `IBugChange`."""
+        return {
+            'text': (
+                "** No longer affects: %s" % self.targetname),
+            }
+
+
 class SeriesNominated(BugChangeBase):
     """A user nominated the bug to be fixed in a series."""
 

=== modified file 'lib/lp/bugs/configure.zcml'
--- lib/lp/bugs/configure.zcml	2011-10-19 23:52:27 +0000
+++ lib/lp/bugs/configure.zcml	2011-10-19 23:52:28 +0000
@@ -1080,6 +1080,9 @@
         for="lp.bugs.interfaces.bugtask.IBugTask                 lazr.lifecycle.interfaces.IObjectCreatedEvent"
         handler="lp.bugs.subscribers.bugactivity.notify_bugtask_added"/>
     <subscriber
+        for="lp.bugs.interfaces.bugtask.IBugTask                 lazr.lifecycle.interfaces.IObjectDeletedEvent"
+        handler="lp.bugs.subscribers.bugactivity.notify_bugtask_deleted"/>
+    <subscriber
         for="lp.bugs.interfaces.bugtask.IBugTask                 zope.lifecycleevent.interfaces.IObjectCreatedEvent"
         handler="canonical.launchpad.subscribers.karma.bugtask_created"/>
     <subscriber

=== added file 'lib/lp/bugs/mail/tests/test_bug_task_deletion.py'
--- lib/lp/bugs/mail/tests/test_bug_task_deletion.py	1970-01-01 00:00:00 +0000
+++ lib/lp/bugs/mail/tests/test_bug_task_deletion.py	2011-10-19 23:52:28 +0000
@@ -0,0 +1,42 @@
+# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Test for emails sent after bug task deletion."""
+
+import transaction
+from zope.component import getUtility
+
+from lp.bugs.model.bugnotification import BugNotification
+from canonical.launchpad.webapp.interfaces import ILaunchBag
+from canonical.testing.layers import DatabaseFunctionalLayer
+from lp.bugs.scripts.bugnotification import construct_email_notifications
+from lp.services.features.testing import FeatureFixture
+from lp.testing import TestCaseWithFactory
+
+
+class TestDeletionNotification(TestCaseWithFactory):
+    """Test email notifications when a bug task is deleted."""
+
+    layer = DatabaseFunctionalLayer
+
+    def setUp(self):
+        # Run the tests as a logged-in user.
+        super(TestDeletionNotification, self).setUp(
+            user='[e-mail]')
+        self.user = getUtility(ILaunchBag).user
+        product = self.factory.makeProduct(owner=self.user)
+        self.bug_task = self.factory.makeBugTask(target=product)
+
+    def test_for_bug_modifier_header(self):
+        """Test X-Launchpad-Bug-Modifier appears when a bugtask is deleted."""
+        flags = {u"disclosure.delete_bugtask.enabled": u"on"}
+        with FeatureFixture(flags):
+            self.bug_task.delete(self.user)
+        transaction.commit()
+        latest_notification = BugNotification.selectFirst(orderBy='-id')
+        notifications, omitted, messages = construct_email_notifications(
+            [latest_notification])
+        self.assertEqual(len(notifications), 1,
+                         'email notification not created')
+        headers = [msg['X-Launchpad-Bug-Modifier'] for msg in messages]
+        self.assertEqual(len(headers), len(messages))

=== modified file 'lib/lp/bugs/model/bugtask.py'
--- lib/lp/bugs/model/bugtask.py	2011-10-19 23:52:27 +0000
+++ lib/lp/bugs/model/bugtask.py	2011-10-19 23:52:28 +0000
@@ -29,7 +29,10 @@
 import re
 
 from lazr.enum import BaseItem
-from lazr.lifecycle.event import ObjectModifiedEvent
+from lazr.lifecycle.event import (
+    ObjectDeletedEvent,
+    ObjectModifiedEvent,
+    )
 from lazr.lifecycle.snapshot import Snapshot
 import pytz
 from sqlobject import (
@@ -637,6 +640,7 @@
                 "Cannot delete bugtask: %s" % self.title)
         bug = self.bug
         target = self.target
+        notify(ObjectDeletedEvent(self, who))
         self.destroySelf()
         del get_property_cache(bug).bugtasks
 

=== modified file 'lib/lp/bugs/model/tests/test_bugtask.py'
--- lib/lp/bugs/model/tests/test_bugtask.py	2011-10-19 23:52:27 +0000
+++ lib/lp/bugs/model/tests/test_bugtask.py	2011-10-19 23:52:28 +0000
@@ -9,11 +9,8 @@
 
 from lazr.lifecycle.event import ObjectModifiedEvent
 from lazr.lifecycle.snapshot import Snapshot
-<<<<<<< TREE
+from lazr.restfulclient.errors import Unauthorized
 from testtools.testcase import ExpectedException
-=======
-from lazr.restfulclient.errors import Unauthorized
->>>>>>> MERGE-SOURCE
 from testtools.matchers import Equals
 from zope.component import getUtility
 from zope.event import notify

=== modified file 'lib/lp/bugs/subscribers/bugactivity.py'
--- lib/lp/bugs/subscribers/bugactivity.py	2011-02-11 22:04:25 +0000
+++ lib/lp/bugs/subscribers/bugactivity.py	2011-10-19 23:52:28 +0000
@@ -14,6 +14,7 @@
 from canonical.database.sqlbase import block_implicit_flushes
 from lp.bugs.adapters.bugchange import (
     BugTaskAdded,
+    BugTaskDeleted,
     BugWatchAdded,
     BugWatchRemoved,
     CveLinkedToBug,
@@ -96,11 +97,11 @@
 @block_implicit_flushes
 def record_bug_added(bug, object_created_event):
     activity = getUtility(IBugActivitySet).new(
-        bug = bug.id,
-        datechanged = UTC_NOW,
-        person = IPerson(object_created_event.user),
-        whatchanged = "bug",
-        message = "added bug")
+        bug=bug.id,
+        datechanged=UTC_NOW,
+        person=IPerson(object_created_event.user),
+        whatchanged="bug",
+        message="added bug")
     bug.addCommentNotification(bug.initial_message, activity=activity)
 
 
@@ -165,6 +166,16 @@
 
 
 @block_implicit_flushes
+def notify_bugtask_deleted(bugtask, event):
+    """A bugtask has been deleted (removed from a bug).
+
+    bugtask must be in IBugTask. event must be anIObjectDeletedEvent.
+    """
+    bugtask.bug.addChange(
+        BugTaskDeleted(UTC_NOW, IPerson(event.user), bugtask))
+
+
+@block_implicit_flushes
 def notify_bug_watch_modified(modified_bug_watch, event):
     """Notify CC'd bug subscribers that a bug watch was edited.
 

=== modified file 'lib/lp/bugs/tests/test_bugchanges.py'
--- lib/lp/bugs/tests/test_bugchanges.py	2011-10-05 04:14:04 +0000
+++ lib/lp/bugs/tests/test_bugchanges.py	2011-10-19 23:52:28 +0000
@@ -25,7 +25,9 @@
 from lp.bugs.interfaces.cve import ICveSet
 from lp.bugs.model.bugnotification import BugNotification
 from lp.bugs.scripts.bugnotification import construct_email_notifications
+from lp.services.features.testing import FeatureFixture
 from lp.testing import (
+    login_person,
     person_logged_in,
     TestCaseWithFactory,
     )
@@ -1403,6 +1405,34 @@
             expected_activity=expected_activity,
             expected_notification=expected_notification)
 
+    def test_bugtask_deleted(self):
+        # Deleting a bug task adds entries in both BugActivity and
+        # BugNotification.
+        target = self.factory.makeProduct()
+        task_to_delete = self.bug.addTask(self.user, target)
+        self.saveOldChanges()
+
+        login_person(self.user)
+        flags = {u"disclosure.delete_bugtask.enabled": u"on"}
+        with FeatureFixture(flags):
+            task_to_delete.delete()
+
+        task_deleted_activity = {
+            'person': self.user,
+            'whatchanged': 'bug task deleted',
+            'oldvalue': target.bugtargetname,
+            }
+
+        task_deleted_notification = {
+            'person': self.user,
+            'text': (
+                "** No longer affects: %s" % target.bugtargetname),
+            }
+
+        self.assertRecordedChange(
+            expected_notification=task_deleted_notification,
+            expected_activity=task_deleted_activity)
+
     def test_product_series_nominated(self):
         # Nominating a bug to be fixed in a product series adds an item
         # to the activity log only.