launchpad-reviewers team mailing list archive

[Graham Binns <graham@xxxxxxxxxxxxx>]

Graham Binns has proposed merging lp:~gmb/launchpad/fix-update-bug-watch into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  #568505 BugWatchUpdater.updateBugWatch() shouldn't need 10 parameters
  https://bugs.launchpad.net/bugs/568505
  #578714 BugWatchUpdater.updateBugWatch() shouldn't take can_foo parameters
  https://bugs.launchpad.net/bugs/578714


This branch fixes the linked bugs by removing the extraneous parameters
from BugWatchUpdater.updateBugWatch(). I've also altered the tests for
that method.
-- 
https://code.launchpad.net/~gmb/launchpad/fix-update-bug-watch/+merge/43643
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~gmb/launchpad/fix-update-bug-watch into lp:launchpad.

=== modified file 'lib/lp/bugs/scripts/checkwatches/bugwatchupdater.py'
--- lib/lp/bugs/scripts/checkwatches/bugwatchupdater.py	2010-10-26 15:47:24 +0000
+++ lib/lp/bugs/scripts/checkwatches/bugwatchupdater.py	2010-12-14 13:56:16 +0000
@@ -53,14 +53,9 @@
         self.can_push_comments = parent.can_push_comments
         self.can_back_link = parent.can_back_link
 
-    # XXX 2010-05-11 gmb bug=578714:
-    #     The last three parameters on this method aren't needed and
-    #     should be removed.
     @commit_before
     def updateBugWatch(self, new_remote_status, new_malone_status,
-                       new_remote_importance, new_malone_importance,
-                       can_import_comments=None, can_push_comments=None,
-                       can_back_link=None):
+                       new_remote_importance, new_malone_importance):
         """Update the BugWatch."""
         with self.transaction:
             if new_malone_status is not None:
@@ -88,14 +83,14 @@
         oops_id = None
         if do_sync:
             try:
-                if can_import_comments or self.can_import_comments:
+                if self.can_import_comments:
                     error_status = (
                         BugWatchActivityStatus.COMMENT_IMPORT_FAILED)
                     self.importBugComments()
-                if can_push_comments or self.can_push_comments:
+                if self.can_push_comments:
                     error_status = BugWatchActivityStatus.COMMENT_PUSH_FAILED
                     self.pushBugComments()
-                if can_back_link or self.can_back_link:
+                if self.can_back_link:
                     error_status = BugWatchActivityStatus.BACKLINK_FAILED
                     self.linkLaunchpadBug()
             except Exception, ex:

=== modified file 'lib/lp/bugs/scripts/checkwatches/tests/test_bugwatchupdater.py'
--- lib/lp/bugs/scripts/checkwatches/tests/test_bugwatchupdater.py	2010-10-04 19:50:45 +0000
+++ lib/lp/bugs/scripts/checkwatches/tests/test_bugwatchupdater.py	2010-12-14 13:56:16 +0000
@@ -25,7 +25,9 @@
 
 
 def make_bug_watch_updater(checkwatches_master, bug_watch,
-                           external_bugtracker, server_time=None):
+                           external_bugtracker, server_time=None,
+                           can_import_comments=False,
+                           can_push_comments=False, can_back_link=False):
     """Helper function to create a BugWatchUpdater instance."""
     if server_time is None:
         server_time = datetime.now()
@@ -33,10 +35,17 @@
     remote_bug_updater = checkwatches_master.remote_bug_updater_factory(
         checkwatches_master, external_bugtracker, bug_watch.remotebug,
         [bug_watch.id], [], server_time)
-    return BugWatchUpdater(
+
+    bug_watch_updater = BugWatchUpdater(
         remote_bug_updater, bug_watch,
         remote_bug_updater.external_bugtracker)
 
+    bug_watch_updater.can_import_comments = can_import_comments
+    bug_watch_updater.can_push_comments = can_push_comments
+    bug_watch_updater.can_back_link = can_back_link
+
+    return bug_watch_updater
+
 
 class BrokenCommentSyncingExternalBugTracker(TestExternalBugTracker):
     """An ExternalBugTracker that can't sync comments."""
@@ -100,8 +109,7 @@
 
         bug_watch_updater.updateBugWatch(
             'FIXED', BugTaskStatus.FIXRELEASED, 'LOW',
-            BugTaskImportance.LOW, can_import_comments=False,
-            can_push_comments=False, can_back_link=False)
+            BugTaskImportance.LOW)
 
         self.assertEqual('FIXED', self.bug_watch.remotestatus)
         self.assertEqual(BugTaskStatus.FIXRELEASED, self.bug_task.status)
@@ -119,12 +127,12 @@
         external_bugtracker = BrokenCommentSyncingExternalBugTracker(
             'http://example.com')
         bug_watch_updater = make_bug_watch_updater(
-            self.checkwatches_master, self.bug_watch, external_bugtracker)
+            self.checkwatches_master, self.bug_watch, external_bugtracker,
+            can_import_comments=True)
 
         bug_watch_updater.updateBugWatch(
             'FIXED', BugTaskStatus.FIXRELEASED, 'LOW',
-            BugTaskImportance.LOW, can_import_comments=True,
-            can_push_comments=False, can_back_link=False)
+            BugTaskImportance.LOW)
 
         self._checkLastErrorAndMessage(
             BugWatchActivityStatus.COMMENT_IMPORT_FAILED,
@@ -136,15 +144,15 @@
         external_bugtracker = BrokenCommentSyncingExternalBugTracker(
             'http://example.com')
         bug_watch_updater = make_bug_watch_updater(
-            self.checkwatches_master, self.bug_watch, external_bugtracker)
+            self.checkwatches_master, self.bug_watch, external_bugtracker,
+            can_push_comments=True)
 
         self.factory.makeBugComment(
             bug=self.bug_task.bug, bug_watch=self.bug_watch)
 
         bug_watch_updater.updateBugWatch(
             'FIXED', BugTaskStatus.FIXRELEASED, 'LOW',
-            BugTaskImportance.LOW, can_import_comments=False,
-            can_push_comments=True, can_back_link=False)
+            BugTaskImportance.LOW)
 
         self._checkLastErrorAndMessage(
             BugWatchActivityStatus.COMMENT_PUSH_FAILED,
@@ -156,12 +164,12 @@
         external_bugtracker = BrokenCommentSyncingExternalBugTracker(
             'http://example.com')
         bug_watch_updater = make_bug_watch_updater(
-            self.checkwatches_master, self.bug_watch, external_bugtracker)
+            self.checkwatches_master, self.bug_watch, external_bugtracker,
+            can_back_link=True)
 
         bug_watch_updater.updateBugWatch(
             'FIXED', BugTaskStatus.FIXRELEASED, 'LOW',
-            BugTaskImportance.LOW, can_import_comments=False,
-            can_push_comments=False, can_back_link=True)
+            BugTaskImportance.LOW)
 
         self._checkLastErrorAndMessage(
             BugWatchActivityStatus.BACKLINK_FAILED,