launchpad-reviewers team mailing list archive

[Aaron Bentley <aaron@xxxxxxxxxxxxx>]

Aaron Bentley has proposed merging lp:~abentley/launchpad/mark-dup-empty into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #811262 in Launchpad itself: "AttributeError: 'NoneType' object has no attribute 'updateHeat' "
  https://bugs.launchpad.net/launchpad/+bug/811262

For more details, see:
https://code.launchpad.net/~abentley/launchpad/mark-dup-empty/+merge/69704

= Summary =
Fix bug #811262: AttributeError: 'NoneType' object has no attribute 'updateHeat'

== Proposed fix ==
Do not assume that the current duplicate is non-None if markAsDuplicate is called with None.

== Pre-implementation notes ==
None

== Implementation details ==
The code assumed the bug must be a duplicate if markAsDuplicate(None) was called.  I changed it to check for None.

== Tests ==
bin/test test_bug -t markAsD

== Demo and Q/A ==
Go to a bug's mark-as-duplicate page for a bug that is not alredy marked as a duplicate.  Don't enter a value, and hit 'Change'.  You should not get an oops.

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/bugs/model/tests/test_bug.py
  lib/lp/bugs/model/bug.py
-- 
https://code.launchpad.net/~abentley/launchpad/mark-dup-empty/+merge/69704
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~abentley/launchpad/mark-dup-empty into lp:launchpad.

=== modified file 'lib/lp/bugs/model/bug.py'
--- lib/lp/bugs/model/bug.py	2011-07-22 00:28:34 +0000
+++ lib/lp/bugs/model/bug.py	2011-07-28 18:34:31 +0000
@@ -1863,7 +1863,8 @@
             # from having been a duplicate. We also update the bug that
             # was previously duplicated.
             self.updateHeat(affected_targets)
-            current_duplicateof.updateHeat(affected_targets)
+            if current_duplicateof is not None:
+                current_duplicateof.updateHeat(affected_targets)
         return affected_targets
 
     def markAsDuplicate(self, duplicate_of):

=== modified file 'lib/lp/bugs/model/tests/test_bug.py'
--- lib/lp/bugs/model/tests/test_bug.py	2011-07-22 00:28:34 +0000
+++ lib/lp/bugs/model/tests/test_bug.py	2011-07-28 18:34:31 +0000
@@ -3,6 +3,8 @@
 
 __metaclass__ = type
 
+from testtools.testcase import ExpectedException
+
 from canonical.testing.layers import DatabaseFunctionalLayer
 from lp.bugs.enum import BugNotificationLevel
 from lp.bugs.interfaces.bugtask import BugTaskStatus
@@ -21,6 +23,15 @@
 
     layer = DatabaseFunctionalLayer
 
+    def test_markAsDuplicate_None(self):
+        # Calling markAsDuplicate(None) on a bug that is not currently a
+        # duplicate works correctly, and does not raise an AttributeError.
+        bug = self.factory.makeBug()
+        with ExpectedException(AssertionError, 'AttributeError not raised'):
+            with ExpectedException(AttributeError, ''):
+                with person_logged_in(self.factory.makePerson()):
+                    bug.markAsDuplicate(None)
+
     def test_get_subscribers_for_person_unsubscribed(self):
         bug = self.factory.makeBug()
         person = self.factory.makePerson()