Merge lp:~stevenk/launchpad/deal-with-unsynched-bugwatch-comments into lp:launchpad

[Steve Kowalik]

Also check for unsynchronized messages when checking if the Delete Bug Watch button can be displayed in the UI.

When deleting a bugwatch that has a message which does not contain a remote ID, it is shown in the UI as unsynchronized, and if it is the only bugmessage that references that bugwatch when that bugwatch is deleted, an OOPS will result due to a foreign key violation. I have dealt with that by adding a new method onto IBugWatch, and making use of it in the view.

This branch does not close the linked bug entirely, but it destroys the cause of the OOPS, so it can drop from Critical to Low.

I have destroyed an evil doctest for LoC credit, as well as refactoring the current method on IBugWatch.

[Ian Booth (wallyworld)]

review: Approve

=== modified file 'Makefile'
--- Makefile	2012-09-24 17:14:32 +0000
+++ Makefile	2012-10-25 00:28:22 +0000
@@ -132,9 +132,6 @@
 logs:
 	mkdir logs
 
-xxxreport: $(PY)
-	${PY} -t ./utilities/xxxreport.py -f csv -o xxx-report.csv ./
-
 codehosting-dir:
 	mkdir -p $(CODEHOSTING_ROOT)/mirrors
 	mkdir -p $(CODEHOSTING_ROOT)/config

=== modified file 'lib/lp/bugs/browser/bugwatch.py'
--- lib/lp/bugs/browser/bugwatch.py	2011-12-30 06:14:56 +0000
+++ lib/lp/bugs/browser/bugwatch.py	2012-10-25 00:28:22 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2011 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """IBugWatch-related browser views."""
@@ -8,8 +8,8 @@
     'BugWatchSetNavigation',
     'BugWatchActivityPortletView',
     'BugWatchEditView',
-    'BugWatchView']
-
+    'BugWatchView'
+    ]
 
 from zope.component import getUtility
 from zope.interface import Interface
@@ -133,7 +133,7 @@
         """Return whether the bug watch is unlinked."""
         return (
             len(self.context.bugtasks) == 0 and
-            self.context.getImportedBugMessages().is_empty())
+            self.context.getBugMessages().is_empty())
 
     @action('Delete Bug Watch', name='delete', condition=bugWatchIsUnlinked)
     def delete_action(self, action, data):

=== modified file 'lib/lp/bugs/browser/tests/test_bugwatch_views.py'
--- lib/lp/bugs/browser/tests/test_bugwatch_views.py	2012-01-01 02:58:52 +0000
+++ lib/lp/bugs/browser/tests/test_bugwatch_views.py	2012-10-25 00:28:22 +0000
@@ -1,4 +1,4 @@
-# Copyright 2010-2011 Canonical Ltd.  This software is licensed under the
+# Copyright 2010-2012 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Tests for BugWatch views."""
@@ -7,7 +7,10 @@
 
 from zope.component import getUtility
 
+from lp.app.errors import NotFoundError
+from lp.bugs.interfaces.bugwatch import IBugWatchSet
 from lp.services.messages.interfaces.message import IMessageSet
+from lp.services.webapp.interfaces import ILaunchBag
 from lp.testing import (
     login,
     login_person,
@@ -32,15 +35,29 @@
         self.bug_watch = self.factory.makeBugWatch(
             bug=self.bug_task.bug)
 
+    def test_can_delete_watch(self):
+        # An unlinked bugwatch can be deleted.
+        bwid = self.bug_watch.id
+        form = {'field.actions.delete': 'Delete Bug Watch'}
+        getUtility(ILaunchBag).add(self.bug_task.bug)
+        view = create_initialized_view(self.bug_watch, '+edit', form=form)
+        self.assertContentEqual([], view.errors)
+        self.assertRaises(NotFoundError, getUtility(IBugWatchSet).get, bwid)
+
+    def test_can_not_delete_unlinked_watch_with_unsynched_comments(self):
+        # If a bugwatch is unlinked, but has imported comments that are
+        # awaiting synch, it can not be deleted.
+        self.factory.makeBugComment(
+            bug=self.bug_task.bug.id, bug_watch=self.bug_watch)
+        view = create_initialized_view(self.bug_watch, '+edit')
+        self.assertFalse(view.bugWatchIsUnlinked(None))
+
     def test_cannot_delete_watch_if_linked_to_task(self):
         # It isn't possible to delete a bug watch that's linked to a bug
         # task.
         self.bug_task.bugwatch = self.bug_watch
         view = create_initialized_view(self.bug_watch, '+edit')
-        self.assertFalse(
-            view.bugWatchIsUnlinked(None),
-            "bugWatchIsUnlinked() returned True though there is a task "
-            "linked to the watch.")
+        self.assertFalse(view.bugWatchIsUnlinked(None))
 
     def test_cannot_delete_watch_if_linked_to_comment(self):
         # It isn't possible to delete a bug watch that's linked to a bug
@@ -54,7 +71,4 @@
         self.bug_watch.addComment('comment-id', message)
         login_person(self.person)
         view = create_initialized_view(self.bug_watch, '+edit')
-        self.assertFalse(
-            view.bugWatchIsUnlinked(None),
-            "bugWatchIsUnlinked() returned True though there is a comment "
-            "linked to the watch.")
+        self.assertFalse(view.bugWatchIsUnlinked(None))

=== modified file 'lib/lp/bugs/configure.zcml'
--- lib/lp/bugs/configure.zcml	2012-10-15 02:32:30 +0000
+++ lib/lp/bugs/configure.zcml	2012-10-25 00:28:22 +0000
@@ -879,6 +879,7 @@
                     remotestatus
                     title
                     url
+                    getBugMessages
                     getImportedBugMessages"/>
             <require
                 permission="launchpad.AnyPerson"

=== modified file 'lib/lp/bugs/interfaces/bugwatch.py'
--- lib/lp/bugs/interfaces/bugwatch.py	2012-10-09 05:48:57 +0000
+++ lib/lp/bugs/interfaces/bugwatch.py	2012-10-25 00:28:22 +0000
@@ -263,6 +263,12 @@
         :param message: The imported comment as a Launchpad Message object.
         """
 
+    def getBugMessages(clauses):
+        """Return all the `IBugMessage`s that reference this BugWatch.
+        
+        :param clauses: A iterable of Storm clauses to limit the messages.
+        """
+
     def getImportedBugMessages():
         """Return all the `IBugMessage`s that have been imported."""
 

=== modified file 'lib/lp/bugs/model/bugwatch.py'
--- lib/lp/bugs/model/bugwatch.py	2012-10-09 04:47:36 +0000
+++ lib/lp/bugs/model/bugwatch.py	2012-10-25 00:28:22 +0000
@@ -283,19 +283,14 @@
             remote_comment_id=comment_id)
         return bug_message
 
+    def getBugMessages(self, clauses=[]):
+        return Store.of(self).find(
+            BugMessage, BugMessage.bug == self.bug.id,
+            BugMessage.bugwatch == self.id, *clauses)
+
     def getImportedBugMessages(self):
         """See `IBugWatch`."""
-        store = Store.of(self)
-        # If a comment is linked to a bug watch and has a
-        # remote_comment_id, it means it's imported.
-        # XXX gmb 2008-12-09 bug 244768:
-        #     The Not() needs to be in this find() call due to bug
-        #     244768; we should remove it once that is solved.
-        return store.find(
-            BugMessage,
-            BugMessage.bug == self.bug.id,
-            BugMessage.bugwatch == self.id,
-            Not(BugMessage.remote_comment_id == None))
+        return self.getBugMessages([BugMessage.remote_comment_id != None])
 
     def addActivity(self, result=None, message=None, oops_id=None):
         """See `IBugWatch`."""

=== removed file 'lib/lp/bugs/stories/bugwatches/xx-delete-bugwatch.txt'
--- lib/lp/bugs/stories/bugwatches/xx-delete-bugwatch.txt	2010-09-29 11:07:13 +0000
+++ lib/lp/bugs/stories/bugwatches/xx-delete-bugwatch.txt	1970-01-01 00:00:00 +0000
@@ -1,34 +0,0 @@
-= Delete a Bug Watch =
-
-If a bug watch isn't linked to a bug task and no comments have been
-imported for it, it's possible to delete from the bug watch edit page.
-
-    >>> user_browser.open('http://launchpad.dev/bugs/1')
-    >>> bug_watches = find_portlet(user_browser.contents, 'Remote bug watches')
-    >>> for li in bug_watches('li'):
-    ...     print li.findAll('a')[0].renderContents()
-    mozilla.org #123543
-    mozilla.org #2000
-    mozilla.org #42
-    debbugs #304014
-
-    >>> user_browser.getLink(url='/+watch/3').click()
-    >>> user_browser.title
-    'Edit bug watch for bug 123543 in The Mozilla.org Bug Tracker on bug #1'
-    >>> print find_portlet(
-    ...     user_browser.contents, 'Bug watch links').span.renderContents()
-    There are currently no links to this bug watch.
-
-    >>> user_browser.getControl('Delete Bug Watch').click()
-
-    >>> for message in find_tags_by_class(user_browser.contents, 'message'):
-    ...     print message.renderContents()
-    The <a href="https://bugzilla.mozilla.org/...123543">mozilla.org #123543</a>
-    bug watch has been deleted.
-
-    >>> bug_watches = find_portlet(user_browser.contents, 'Remote bug watches')
-    >>> for li in bug_watches('li'):
-    ...     print li.findAll('a')[0].renderContents()
-    mozilla.org #2000
-    mozilla.org #42
-    debbugs #304014