launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

William Grant has proposed merging lp:~wgrant/launchpad/transitionToTarget-validate_target into lp:launchpad with lp:~wgrant/launchpad/sensible-validate_target as a prerequisite.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~wgrant/launchpad/transitionToTarget-validate_target/+merge/69189

This branch extends transitionToTarget to call validate_target in addition to its existing checks. This brings it into line with the validation performed by BugTaskEditView. A couple of tests needed fixing to comply with these restrictions: test_milestone and xx-bug.txt attempted to move targets to unpublished packages.

transitionToTarget also no longer unsets the milestone unless the transition checks succeed.
-- 
https://code.launchpad.net/~wgrant/launchpad/transitionToTarget-validate_target/+merge/69189
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/transitionToTarget-validate_target into lp:launchpad.

=== modified file 'lib/lp/bugs/model/bugtask.py'
--- lib/lp/bugs/model/bugtask.py	2011-07-26 01:16:08 +0000
+++ lib/lp/bugs/model/bugtask.py	2011-07-26 01:16:10 +0000
@@ -1147,15 +1147,11 @@
         lib/canonical/launchpad/browser/bugtask.py#BugTaskEditView.
         """
 
+        if self.target == target:
+            return
+
         target_before_change = self.target
 
-        if (self.milestone is not None and
-            self.milestone.target != target):
-            # If the milestone for this bugtask is set, we
-            # have to make sure that it's a milestone of the
-            # current target, or reset it to None
-            self.milestone = None
-
         # Check if any series are involved. You can't retarget series
         # tasks. Except for SourcePackage tasks, which can only be
         # retargetted to another SourcePackage in the same DistroSeries.
@@ -1172,6 +1168,8 @@
                     "Series source package tasks may only be retargetted "
                     "to another source package in the same series.")
 
+        validate_target(self.bug, target)
+
         # Inhibit validate_target_attribute, as we can't set them all
         # atomically, but we know the final result is correct.
         self._inhibit_target_check = True
@@ -1180,6 +1178,13 @@
         self._inhibit_target_check = False
         self.updateTargetNameCache()
 
+        if (self.milestone is not None and
+            self.milestone.target != target):
+            # If the milestone for this bugtask is set, we
+            # have to make sure that it's a milestone of the
+            # current target, or reset it to None
+            self.milestone = None
+
         # After the target has changed, we need to recalculate the maximum bug
         # heat for the new and old targets.
         if self.target != target_before_change:

=== modified file 'lib/lp/bugs/model/tests/test_bugtask.py'
--- lib/lp/bugs/model/tests/test_bugtask.py	2011-07-26 01:16:08 +0000
+++ lib/lp/bugs/model/tests/test_bugtask.py	2011-07-26 01:16:10 +0000
@@ -1744,6 +1744,32 @@
         sp2 = self.factory.makeSourcePackage(distroseries=ds2)
         self.assertTransitionForbidden(sp1, sp2)
 
+    def test_transition_to_same_is_noop(self):
+        p = self.factory.makeProduct()
+        self.assertTransitionWorks(p, p)
+
+    def test_validate_target_is_called(self):
+        p = self.factory.makeProduct()
+        task1 = self.factory.makeBugTask(target=p)
+        task2 = self.factory.makeBugTask(
+            bug=task1.bug, target=self.factory.makeProduct())
+        with person_logged_in(task2.owner):
+            self.assertRaisesWithContent(
+                IllegalTarget,
+                "A fix for this bug has already been requested for %s"
+                % p.displayname, task2.transitionToTarget, p)
+
+    def test_milestone_preserved_if_transition_rejected(self):
+        series = self.factory.makeProductSeries()
+        task = self.factory.makeBugTask(target=series.product)
+        milestone = self.factory.makeMilestone(product=series.product)
+        with person_logged_in(task.owner):
+            task.milestone = milestone
+            self.assertRaises(
+                IllegalTarget,
+                task.transitionToTarget, self.factory.makeSourcePackage())
+            self.assertEqual(milestone, task.milestone)
+
 
 class TestBugTargetKeys(TestCaseWithFactory):
     """Tests for bug_target_to_key and bug_target_from_key."""

=== modified file 'lib/lp/bugs/stories/webservice/xx-bug.txt'
--- lib/lp/bugs/stories/webservice/xx-bug.txt	2011-07-21 07:58:15 +0000
+++ lib/lp/bugs/stories/webservice/xx-bug.txt	2011-07-26 01:16:10 +0000
@@ -448,8 +448,15 @@
     http://.../debian/+milestone/3.1
 
 We can change the task's target. Here we change the task's target from
-the mozilla-firefox package to alsa-utils.
+the mozilla-firefox package to alsa-utils. Only published packages can
+have tasks, so we first add a publication.
 
+    >>> from lp.registry.interfaces.distribution import IDistributionSet
+    >>> login('admin@xxxxxxxxxxxxx')
+    >>> debian = getUtility(IDistributionSet).getByName('debian')
+    >>> ignored = factory.makeSourcePackagePublishingHistory(
+    ...     distroseries=debian.currentseries, sourcepackagename='evolution')
+    >>> logout()
     >>> print webservice.named_post(
     ...     task['self_link'], 'transitionToTarget',
     ...     target=webservice.getAbsoluteUrl('/debian/+source/evolution'))
@@ -526,7 +533,6 @@
 We can change a distribution task to a task with a package from the same
 distribution.
 
-    >>> from lp.registry.interfaces.distribution import IDistributionSet
     >>> login('foo.bar@xxxxxxxxxxxxx')
     >>> distro_bugtask = factory.makeBugTask(
     ...     target=getUtility(IDistributionSet).getByName('ubuntu'))

=== modified file 'lib/lp/registry/browser/tests/test_milestone.py'
--- lib/lp/registry/browser/tests/test_milestone.py	2011-05-27 21:12:25 +0000
+++ lib/lp/registry/browser/tests/test_milestone.py	2011-07-26 01:16:10 +0000
@@ -397,6 +397,9 @@
             bug = self.factory.makeBug(distribution=self.ubuntu)
             distrosourcepackage = self.factory.makeDistributionSourcePackage(
                 distribution=self.ubuntu)
+            self.factory.makeSourcePackagePublishingHistory(
+                distroseries=self.ubuntu.currentseries,
+                sourcepackagename=distrosourcepackage.sourcepackagename)
             bug.bugtasks[0].transitionToTarget(distrosourcepackage)
             bug.bugtasks[0].transitionToMilestone(
                 self.milestone, self.owner)