Merge lp:~wgrant/launchpad/sensible-validate_target into lp:launchpad

[William Grant]

Replace validate_distrotask and valid_upstreamtask with a more general and sensible validate_target.

This branch continues my ruthless campaign to move BugTask.target handling completely into the model. It takes validate_distrotask and valid_upstreamtask, combining them into a single validate_target which can be easily called by transitionToTarget. It now lives in lp.bugs, and has extensive unittests rather than doctests.

[Curtis Hovey (sinzui)]

This branch looks good to land. I have a comment that relates to my own recent work. Maybe your thoughts will help us to make a method faster.

> === modified file 'lib/lp/bugs/model/bugtask.py'
> --- lib/lp/bugs/model/bugtask.py 2011-07-22 04:50:51 +0000
> +++ lib/lp/bugs/model/bugtask.py 2011-07-25 23:06:42 +0000
...
> @@ -439,6 +437,65 @@
> return validate_target_attribute(self, attr, value)
>
>
> +def validate_target(bug, target):
> + """Validate a bugtask target against a bug's existing tasks.
> +
> + Checks that no conflicting tasks already exist.
> + """
> + if bug.getBugTask(target):
> + raise IllegalTarget(
> + "A fix for this bug has already been requested for %s"
> + % target.displayname)
> +
> + if IDistributionSourcePackage.providedBy(target):
> + # If the distribution has at least one series, check that the
> + # source package has been published in the distribution.
> + if (target.sourcepackagename is not None and
> + len(target.distribution.series) > 0):
> + try:
> + target.distribution.guessPublishedSourcePackageName(
> + target.sourcepackagename.name)

I believe this is the method that contributes to bugs
https://bugs.launchpad.net/launchpad/+bug/618366 and
https://bugs.launchpad.net/launchpad/+bug/721643

I tried to avoid using it. Do you know why guessPublishedSourcePackageName()
requires a string and does the extra work to get the spn. We already have
an spn half the time. I also want to avoid calling this method if the method
that provided the spn can be trusted to have provide one that was published
in the distro.

Ah, but this method is smart enough to verify the branch for the package.
I like that.

The binary name is in part the fault of the existing vocabs. I hope
Steven's work will make this path in the method obsolete, or at least very
rare.

review: Approve (code)

=== modified file 'lib/canonical/launchpad/doc/validation.txt'
--- lib/canonical/launchpad/doc/validation.txt	2011-02-28 20:46:41 +0000
+++ lib/canonical/launchpad/doc/validation.txt	2011-07-25 23:06:42 +0000
@@ -1,110 +1,5 @@
 = Validation =
 
-The validation interface contains different kinds of validation functions.
-
-== validate_distrotask() ==
-
-The validate_distrotask() function is used to guarantee that distribution
-bugtasks are unique per bug.
-
-    >>> from canonical.launchpad.interfaces.validation import validate_distrotask
-    >>> from lp.bugs.interfaces.bug import IBugSet
-    >>> from lp.registry.interfaces.distribution import IDistributionSet
-    >>> from lp.registry.interfaces.sourcepackagename import ISourcePackageNameSet
-    >>> bug_two = getUtility(IBugSet).get(2)
-    >>> debian = getUtility(IDistributionSet).getByName('debian')
-    >>> mozilla_firefox = getUtility(
-    ...     ISourcePackageNameSet).queryByName('mozilla-firefox')
-    >>> alsa_utils = getUtility(
-    ...     ISourcePackageNameSet).queryByName('alsa-utils')
-
-We aren't allowed to have two distrotasks with the same sourcepackage.
-
-    >>> validate_distrotask(bug_two, debian, mozilla_firefox)
-    Traceback (most recent call last):
-      ...
-    LaunchpadValidationError: ...
-
-But it's legal to have two bugtasks with different sourcepackages.
-
-    >>> validate_distrotask(bug_two, debian, alsa_utils)
-
-If the bug already has a distribution task with no source package, it's
-not possible to add a another one.
-
-    >>> from lp.bugs.interfaces.bug import CreateBugParams
-    >>> from lp.registry.interfaces.distribution import IDistributionSet
-
-    >>> login('[e-mail]')
-    >>> no_priv = getUtility(ILaunchBag).user
-    >>> ubuntu = getUtility(IDistributionSet).getByName('ubuntu')
-    >>> ubuntu_bug_no_sourcepackage = ubuntu.createBug(
-    ...     CreateBugParams(no_priv, 'Test Bug', 'This is a test'))
-    >>> validate_distrotask(ubuntu_bug_no_sourcepackage, ubuntu, None)
-    Traceback (most recent call last):
-      ...
-    LaunchpadValidationError: ...
-
-It's also not possible to have a bugtask for a sourcepackage that has
-not been published in the target distribution.
-
-    >>> foobar = getUtility(
-    ...     ISourcePackageNameSet).queryByName('foobar')
-    >>> validate_distrotask(ubuntu_bug_no_sourcepackage, ubuntu, foobar)
-    Traceback (most recent call last):
-    ...
-    LaunchpadValidationError: u'Package foobar not published in Ubuntu'
-
-It's not allowed even if the sourcepackage has been published in a
-PPA.
-
-    >>> from lp.soyuz.enums import PackagePublishingStatus
-    >>> from lp.soyuz.tests.ppa import publishToPPA
-    >>> publishToPPA(
-    ...     person_name='cprov',
-    ...     sourcepackage_name='foobar',
-    ...     sourcepackage_version='1.0',
-    ...     distribution_name='ubuntu',
-    ...     distroseries_name='hoary',
-    ...     publishing_status=PackagePublishingStatus.PUBLISHED)
-
-    >>> validate_distrotask(ubuntu_bug_no_sourcepackage, ubuntu, foobar)
-    Traceback (most recent call last):
-    ...
-    LaunchpadValidationError: u'Package foobar not published in Ubuntu'
-
-Adding a task with a source package is still possible.
-
-    >>> validate_distrotask(ubuntu_bug_no_sourcepackage, ubuntu, alsa_utils)
-
-
-== validate_new_distrotask() ==
-
-The validate_new_distrotask() function does the same checks as
-validate_distrotask(), but it does some extra checks as well.
-
-We can't add a task on just the distribution, because it doesn't
-make sense to have tasks open on just the distribution and also packages
-in that distribution on the same bug.
-
-    >>> from canonical.launchpad.interfaces.validation import (
-    ...     validate_new_distrotask)
-    >>> validate_new_distrotask(bug_two, debian, None)
-    Traceback (most recent call last):
-      ...
-    LaunchpadValidationError: ...
-
-If the bug already has a distribution with no package, we can't add
-another distribution task with a source package (the task with no
-package should be edited instead).
-
-    >>> validate_new_distrotask(
-    ...     ubuntu_bug_no_sourcepackage, ubuntu, alsa_utils)
-    Traceback (most recent call last):
-      ...
-    LaunchpadValidationError: ...
-
-
 == LaunchpadValidationError ==
 
 LaunchpadValidationError is the standard exception used for custom

=== modified file 'lib/canonical/launchpad/interfaces/validation.py'
--- lib/canonical/launchpad/interfaces/validation.py	2011-06-14 20:35:20 +0000
+++ lib/canonical/launchpad/interfaces/validation.py	2011-07-25 23:06:42 +0000
@@ -12,9 +12,6 @@
     'valid_cve_sequence',
     'validate_new_team_email',
     'validate_new_person_email',
-    'validate_distrotask',
-    'validate_new_distrotask',
-    'valid_upstreamtask',
     'valid_password',
     'validate_date_interval',
     ]
@@ -25,8 +22,6 @@
 from zope.app.form.interfaces import WidgetsError
 from zope.component import getUtility
 
-from lazr.restful.error import expose
-
 from canonical.launchpad import _
 from canonical.launchpad.interfaces.emailaddress import (
     IEmailAddressSet,
@@ -150,99 +145,6 @@
     return True
 
 
-def validate_new_distrotask(bug, distribution, sourcepackagename=None):
-    """Validate a distribution bugtask to be added.
-
-    Make sure that the isn't already a distribution task without a
-    source package, or that such task is added only when the bug doesn't
-    already have any tasks for the distribution.
-
-    The same checks as `validate_distrotask` does are also done.
-    """
-    from canonical.launchpad.helpers import shortlist
-
-    if sourcepackagename:
-        # Ensure that there isn't already a generic task open on the
-        # distribution for this bug, because if there were, that task
-        # should be reassigned to the sourcepackage, rather than a new
-        # task opened.
-        if bug.getBugTask(distribution) is not None:
-            raise LaunchpadValidationError(_(
-                    'This bug is already open on ${distribution} with no '
-                    'package specified. You should fill in a package '
-                    'name for the existing bug.',
-                    mapping={'distribution': distribution.displayname}))
-    else:
-        # Prevent having a task on only the distribution if there's at
-        # least one task already on the distribution, whether or not
-        # that task also has a source package.
-        distribution_tasks_for_bug = [
-            bugtask for bugtask
-            in shortlist(bug.bugtasks, longest_expected=50)
-            if bugtask.distribution == distribution]
-
-        if len(distribution_tasks_for_bug) > 0:
-            raise LaunchpadValidationError(_(
-                    'This bug is already on ${distribution}. Please '
-                    'specify an affected package in which the bug '
-                    'has not yet been reported.',
-                    mapping={'distribution': distribution.displayname}))
-    validate_distrotask(bug, distribution, sourcepackagename)
-
-
-def validate_distrotask(bug, distribution, sourcepackagename=None):
-    """Check if a distribution bugtask already exists for a given bug.
-
-    If validation fails, a LaunchpadValidationError will be raised.
-    """
-    if sourcepackagename is not None and len(distribution.series) > 0:
-        # If the distribution has at least one series, check that the
-        # source package has been published in the distribution.
-        try:
-            distribution.guessPublishedSourcePackageName(
-                sourcepackagename.name)
-        except NotFoundError, e:
-            raise LaunchpadValidationError(e)
-    new_source_package = distribution.getSourcePackage(sourcepackagename)
-    if sourcepackagename is not None and (
-        bug.getBugTask(new_source_package) is not None):
-        # Ensure this distribution/sourcepackage task is unique.
-        raise LaunchpadValidationError(_(
-                'This bug has already been reported on ${source} '
-                '(${distribution}).',
-                mapping={'source': sourcepackagename.name,
-                         'distribution': distribution.name}))
-    elif (sourcepackagename is None and
-          bug.getBugTask(distribution) is not None):
-        # Don't allow two distribution tasks with no source package.
-        raise LaunchpadValidationError(_(
-                'This bug has already been reported on ${distribution}.',
-                 mapping={'distribution': distribution.name}))
-    else:
-        # The bugtask is valid.
-        pass
-
-
-def valid_upstreamtask(bug, bug_target):
-    """Check if a bugtask already exists for a given bug/target.
-
-    If it exists, WidgetsError will be raised.
-    """
-    # Local import to avoid circular imports.
-    from lp.bugs.interfaces.bugtask import BugTaskSearchParams
-    errors = []
-    user = getUtility(ILaunchBag).user
-    params = BugTaskSearchParams(user, bug=bug)
-    if not bug_target.searchTasks(params).is_empty():
-        errors.append(
-            LaunchpadValidationError(_(
-                'A fix for this bug has already been requested for ${target}',
-                mapping={'target': bug_target.displayname})))
-
-    if len(errors) > 0:
-        raise expose(WidgetsError(errors), 400)
-
-
 def valid_password(password):
     """Return True if the argument is a valid password.
 

=== modified file 'lib/lp/bugs/browser/bugalsoaffects.py'
--- lib/lp/bugs/browser/bugalsoaffects.py	2011-07-13 02:17:29 +0000
+++ lib/lp/bugs/browser/bugalsoaffects.py	2011-07-25 23:06:42 +0000
@@ -16,10 +16,7 @@
 from lazr.lifecycle.event import ObjectCreatedEvent
 from z3c.ptcompat import ViewPageTemplateFile
 from zope.app.form.browser import DropdownWidget
-from zope.app.form.interfaces import (
-    MissingInputError,
-    WidgetsError,
-    )
+from zope.app.form.interfaces import MissingInputError
 from zope.component import getUtility
 from zope.event import notify
 from zope.formlib import form
@@ -34,10 +31,6 @@
     MultiStepView,
     StepView,
     )
-from canonical.launchpad.interfaces.validation import (
-    valid_upstreamtask,
-    validate_new_distrotask,
-    )
 from canonical.launchpad.webapp import canonical_url
 from canonical.launchpad.webapp.interfaces import ILaunchBag
 from canonical.launchpad.webapp.menu import structured
@@ -62,6 +55,7 @@
     BugTaskStatus,
     IAddBugTaskForm,
     IAddBugTaskWithProductCreationForm,
+    IllegalTarget,
     valid_remote_bug_url,
     )
 from lp.bugs.interfaces.bugtracker import (
@@ -73,6 +67,10 @@
     NoBugTrackerFound,
     UnrecognizedBugTrackerURL,
     )
+from lp.bugs.model.bugtask import (
+    validate_new_target,
+    validate_target,
+    )
 from lp.registry.interfaces.distributionsourcepackage import (
     IDistributionSourcePackage,
     )
@@ -157,8 +155,8 @@
         upstream = bugtask.target.upstream_product
         if upstream is not None:
             try:
-                valid_upstreamtask(bugtask.bug, upstream)
-            except WidgetsError:
+                validate_target(bugtask.bug, upstream)
+            except IllegalTarget:
                 # There is already a task for the upstream.
                 pass
             else:
@@ -173,10 +171,9 @@
     def validateStep(self, data):
         if data.get('product'):
             try:
-                valid_upstreamtask(self.context.bug, data.get('product'))
-            except WidgetsError, errors:
-                for error in errors:
-                    self.setFieldError('product', error.snippet())
+                validate_target(self.context.bug, data.get('product'))
+            except IllegalTarget as e:
+                self.setFieldError('product', e[0])
             return
 
         entered_product = self.request.form.get(self.widgets['product'].name)
@@ -431,10 +428,12 @@
             self.setFieldError('sourcepackagename', error)
         else:
             try:
-                validate_new_distrotask(
-                    self.context.bug, distribution, sourcepackagename)
-            except LaunchpadValidationError, error:
-                self.setFieldError('sourcepackagename', error.snippet())
+                target = distribution
+                if sourcepackagename:
+                    target = target.getSourcePackage(sourcepackagename)
+                validate_new_target(self.context.bug, target)
+            except IllegalTarget as e:
+                self.setFieldError('sourcepackagename', e[0])
 
         super(DistroBugTaskCreationStep, self).validateStep(data)
 
@@ -812,10 +811,9 @@
         self._validate(action, data)
         project = data.get('existing_product')
         try:
-            valid_upstreamtask(self.context.bug, project)
-        except WidgetsError, errors:
-            for error in errors:
-                self.setFieldError('existing_product', error.snippet())
+            validate_target(self.context.bug, project)
+        except IllegalTarget as e:
+            self.setFieldError('existing_product', e[0])
 
     @action('Use Existing Project', name='use_existing_product',
             validator=validate_existing_product)

=== modified file 'lib/lp/bugs/browser/bugtask.py'
--- lib/lp/bugs/browser/bugtask.py	2011-07-21 06:27:29 +0000
+++ lib/lp/bugs/browser/bugtask.py	2011-07-25 23:06:42 +0000
@@ -132,10 +132,6 @@
     FeedsMixin,
     )
 from canonical.launchpad.interfaces.launchpad import IHasExternalBugTracker
-from canonical.launchpad.interfaces.validation import (
-    valid_upstreamtask,
-    validate_distrotask,
-    )
 from canonical.launchpad.mailnotification import get_unified_diff
 from canonical.launchpad.searchbuilder import (
     all,
@@ -238,6 +234,7 @@
     IBugTaskSet,
     ICreateQuestionFromBugTaskForm,
     IFrontPageBugTaskSearch,
+    IllegalTarget,
     INominationsReviewTableBatchNavigator,
     IPersonBugTaskSearch,
     IRemoveQuestionFromBugTaskForm,
@@ -249,6 +246,7 @@
 from lp.bugs.interfaces.bugwatch import BugWatchActivityStatus
 from lp.bugs.interfaces.cve import ICveSet
 from lp.bugs.interfaces.malone import IMaloneApplication
+from lp.bugs.model.bugtask import validate_target
 from lp.registry.interfaces.distribution import (
     IDistribution,
     IDistributionSet,
@@ -1372,15 +1370,20 @@
             distro = bugtask.distroseries.distribution
         else:
             distro = bugtask.distribution
-        sourcename = bugtask.sourcepackagename
         old_product = bugtask.product
 
-        if distro is not None and sourcename != data.get('sourcepackagename'):
+        new_spn = data.get('sourcepackagename')
+        if distro is not None and bugtask.sourcepackagename != new_spn:
             try:
-                validate_distrotask(
-                    bugtask.bug, distro, data.get('sourcepackagename'))
-            except LaunchpadValidationError, error:
-                self.setFieldError('sourcepackagename', str(error))
+                target = distro
+                if new_spn is not None:
+                    target = distro.getSourcePackage(new_spn)
+                validate_target(bugtask.bug, target)
+            except IllegalTarget as e:
+                # The field validator may have already set an error.
+                # Don't clobber it.
+                if not self.getFieldError('sourcepackagename'):
+                    self.setFieldError('sourcepackagename', e[0])
 
         new_product = data.get('product')
         if (old_product is None or old_product == new_product or
@@ -1394,9 +1397,9 @@
             self.setFieldError('product', 'Enter a project name')
         else:
             try:
-                valid_upstreamtask(bugtask.bug, new_product)
-            except WidgetsError, errors:
-                self.setFieldError('product', errors.args[0])
+                validate_target(bugtask.bug, new_product)
+            except IllegalTarget as e:
+                self.setFieldError('product', e[0])
 
     def updateContextFromData(self, data, context=None):
         """Updates the context object using the submitted form data.

=== modified file 'lib/lp/bugs/browser/tests/test_bugtask.py'
--- lib/lp/bugs/browser/tests/test_bugtask.py	2011-07-18 11:32:51 +0000
+++ lib/lp/bugs/browser/tests/test_bugtask.py	2011-07-25 23:06:42 +0000
@@ -705,7 +705,8 @@
             bug_task_2, name='+editstatus', form=form, principal=user)
         self.assertEqual(1, len(view.errors))
         self.assertEqual(
-            'This bug has already been reported on mouse (ubuntu).',
+            'A fix for this bug has already been requested for mouse in '
+            'Ubuntu',
             view.errors[0])
 
     def setUpRetargetMilestone(self):

=== modified file 'lib/lp/bugs/model/bugtask.py'
--- lib/lp/bugs/model/bugtask.py	2011-07-22 04:50:51 +0000
+++ lib/lp/bugs/model/bugtask.py	2011-07-25 23:06:42 +0000
@@ -18,6 +18,8 @@
     'get_bug_privacy_filter',
     'get_related_bugtasks_search_params',
     'search_value_to_where_condition',
+    'validate_new_target',
+    'validate_target',
     ]
 
 
@@ -85,10 +87,6 @@
     )
 from canonical.launchpad.helpers import shortlist
 from canonical.launchpad.interfaces.lpstorm import IStore
-from canonical.launchpad.interfaces.validation import (
-    valid_upstreamtask,
-    validate_new_distrotask,
-    )
 from canonical.launchpad.searchbuilder import (
     all,
     any,
@@ -439,6 +437,65 @@
     return validate_target_attribute(self, attr, value)
 
 
+def validate_target(bug, target):
+    """Validate a bugtask target against a bug's existing tasks.
+
+    Checks that no conflicting tasks already exist.
+    """
+    if bug.getBugTask(target):
+        raise IllegalTarget(
+            "A fix for this bug has already been requested for %s"
+            % target.displayname)
+
+    if IDistributionSourcePackage.providedBy(target):
+        # If the distribution has at least one series, check that the
+        # source package has been published in the distribution.
+        if (target.sourcepackagename is not None and
+            len(target.distribution.series) > 0):
+            try:
+                target.distribution.guessPublishedSourcePackageName(
+                    target.sourcepackagename.name)
+            except NotFoundError, e:
+                raise IllegalTarget(e[0])
+
+
+def validate_new_target(bug, target):
+    """Validate a bugtask target to be added.
+
+    Make sure that the isn't already a distribution task without a
+    source package, or that such task is added only when the bug doesn't
+    already have any tasks for the distribution.
+
+    The same checks as `validate_target` does are also done.
+    """
+    if IDistribution.providedBy(target):
+        # Prevent having a task on only the distribution if there's at
+        # least one task already on the distribution, whether or not
+        # that task also has a source package.
+        distribution_tasks_for_bug = [
+            bugtask for bugtask
+            in shortlist(bug.bugtasks, longest_expected=50)
+            if bugtask.distribution == target]
+
+        if len(distribution_tasks_for_bug) > 0:
+            raise IllegalTarget(
+                "This bug is already on %s. Please specify an "
+                "affected package in which the bug has not yet "
+                "been reported." % target.displayname)
+    elif IDistributionSourcePackage.providedBy(target):
+        # Ensure that there isn't already a generic task open on the
+        # distribution for this bug, because if there were, that task
+        # should be reassigned to the sourcepackage, rather than a new
+        # task opened.
+        if bug.getBugTask(target.distribution) is not None:
+            raise IllegalTarget(
+                "This bug is already open on %s with no package "
+                "specified. You should fill in a package name for "
+                "the existing bug." % target.distribution.displayname)
+
+    validate_target(bug, target)
+
+
 class BugTask(SQLBase):
     """See `IBugTask`."""
     implements(IBugTask)
@@ -2653,7 +2710,8 @@
             elif distribution is not None:
                 # Make sure there's no bug task already filed against
                 # this source package in this distribution.
-                validate_new_distrotask(bug, distribution, sourcepackagename)
+                validate_new_target(
+                    bug, distribution.getSourcePackage(sourcepackagename))
                 stop_checking = True
 
         if target is None and not stop_checking:
@@ -2670,13 +2728,13 @@
                 target = distroseries
             elif distribution is not None and not stop_checking:
                 # Bug filed against a distribution.
-                validate_new_distrotask(bug, distribution)
+                validate_new_target(bug, distribution)
                 stop_checking = True
 
         if target is not None and not stop_checking:
             # Make sure there's no task for this bug already filed
             # against the target.
-            valid_upstreamtask(bug, target)
+            validate_target(bug, target)
 
         if not bug.private and bug.security_related:
             if product and product.security_contact:

=== modified file 'lib/lp/bugs/model/tests/test_bugtask.py'
--- lib/lp/bugs/model/tests/test_bugtask.py	2011-07-22 04:50:51 +0000
+++ lib/lp/bugs/model/tests/test_bugtask.py	2011-07-25 23:06:42 +0000
@@ -42,6 +42,8 @@
     bug_target_to_key,
     build_tag_search_clause,
     IllegalTarget,
+    validate_new_target,
+    validate_target,
     )
 from lp.bugs.tests.bug import (
     create_old_bug,
@@ -57,6 +59,7 @@
     )
 from lp.registry.interfaces.product import IProductSet
 from lp.registry.interfaces.projectgroup import IProjectGroupSet
+from lp.soyuz.interfaces.archive import ArchivePurpose
 from lp.testing import (
     ANONYMOUS,
     EventRecorder,
@@ -1831,3 +1834,177 @@
     def test_no_target_for_bad_keys(self):
         self.assertRaises(
             AssertionError, bug_target_from_key, None, None, None, None, None)
+
+
+class TestValidateTarget(TestCaseWithFactory):
+
+    layer = DatabaseFunctionalLayer
+
+    def test_new_product_is_allowed(self):
+        # A new product not on the bug is OK.
+        p1 = self.factory.makeProduct()
+        task = self.factory.makeBugTask(target=p1)
+        p2 = self.factory.makeProduct()
+        validate_target(task.bug, p2)
+
+    def test_same_product_is_forbidden(self):
+        # A product with an existing task is not.
+        p = self.factory.makeProduct()
+        task = self.factory.makeBugTask(target=p)
+        self.assertRaisesWithContent(
+            IllegalTarget,
+            "A fix for this bug has already been requested for %s"
+            % p.displayname,
+            validate_target, task.bug, p)
+
+    def test_new_distribution_is_allowed(self):
+        # A new distribution not on the bug is OK.
+        d1 = self.factory.makeDistribution()
+        task = self.factory.makeBugTask(target=d1)
+        d2 = self.factory.makeDistribution()
+        validate_target(task.bug, d2)
+
+    def test_new_productseries_is_allowed(self):
+        # A new productseries not on the bug is OK.
+        ds1 = self.factory.makeProductSeries()
+        task = self.factory.makeBugTask(target=ds1)
+        ds2 = self.factory.makeProductSeries()
+        validate_target(task.bug, ds2)
+
+    def test_new_distroseries_is_allowed(self):
+        # A new distroseries not on the bug is OK.
+        ds1 = self.factory.makeDistroSeries()
+        task = self.factory.makeBugTask(target=ds1)
+        ds2 = self.factory.makeDistroSeries()
+        validate_target(task.bug, ds2)
+
+    def test_new_sourcepackage_is_allowed(self):
+        # A new sourcepackage not on the bug is OK.
+        sp1 = self.factory.makeSourcePackage()
+        task = self.factory.makeBugTask(target=sp1)
+        sp2 = self.factory.makeSourcePackage()
+        validate_target(task.bug, sp2)
+
+    def test_multiple_packageless_distribution_tasks_are_forbidden(self):
+        # A distribution with an existing task is not.
+        d = self.factory.makeDistribution()
+        task = self.factory.makeBugTask(target=d)
+        self.assertRaisesWithContent(
+            IllegalTarget,
+            "A fix for this bug has already been requested for %s"
+            % d.displayname,
+            validate_target, task.bug, d)
+
+    def test_distributionsourcepackage_task_is_allowed(self):
+        # A DistributionSourcePackage task can coexist with a task for
+        # its Distribution.
+        d = self.factory.makeDistribution()
+        task = self.factory.makeBugTask(target=d)
+        dsp = self.factory.makeDistributionSourcePackage(distribution=d)
+        validate_target(task.bug, dsp)
+
+    def test_different_distributionsourcepackage_tasks_are_allowed(self):
+        # A DistributionSourcePackage task can also coexist with a task
+        # for another one.
+        dsp1 = self.factory.makeDistributionSourcePackage()
+        task = self.factory.makeBugTask(target=dsp1)
+        dsp2 = self.factory.makeDistributionSourcePackage(
+            distribution=dsp1.distribution)
+        validate_target(task.bug, dsp2)
+
+    def test_same_distributionsourcepackage_task_is_forbidden(self):
+        # But a DistributionSourcePackage task cannot coexist with a
+        # task for itself.
+        dsp = self.factory.makeDistributionSourcePackage()
+        task = self.factory.makeBugTask(target=dsp)
+        self.assertRaisesWithContent(
+            IllegalTarget,
+            "A fix for this bug has already been requested for %s in %s"
+            % (dsp.sourcepackagename.name, dsp.distribution.displayname),
+            validate_target, task.bug, dsp)
+
+    def test_dsp_without_publications_disallowed(self):
+        # If a distribution has series, a DistributionSourcePackage task
+        # can only be created if the package is published in a distro
+        # archive.
+        series = self.factory.makeDistroSeries()
+        dsp = self.factory.makeDistributionSourcePackage(
+            distribution=series.distribution)
+        task = self.factory.makeBugTask()
+        self.assertRaisesWithContent(
+            IllegalTarget,
+            "Package %s not published in %s"
+            % (dsp.sourcepackagename.name, dsp.distribution.displayname),
+            validate_target, task.bug, dsp)
+
+    def test_dsp_with_publications_allowed(self):
+        # If a distribution has series, a DistributionSourcePackage task
+        # can only be created if the package is published in a distro
+        # archive.
+        series = self.factory.makeDistroSeries()
+        dsp = self.factory.makeDistributionSourcePackage(
+            distribution=series.distribution)
+        task = self.factory.makeBugTask()
+        self.factory.makeSourcePackagePublishingHistory(
+            distroseries=series, sourcepackagename=dsp.sourcepackagename,
+            archive=series.main_archive)
+        validate_target(task.bug, dsp)
+
+    def test_dsp_with_only_ppa_publications_disallowed(self):
+        # If a distribution has series, a DistributionSourcePackage task
+        # can only be created if the package is published in a distro
+        # archive. PPA publications don't count.
+        series = self.factory.makeDistroSeries()
+        dsp = self.factory.makeDistributionSourcePackage(
+            distribution=series.distribution)
+        task = self.factory.makeBugTask()
+        self.factory.makeSourcePackagePublishingHistory(
+            distroseries=series, sourcepackagename=dsp.sourcepackagename,
+            archive=self.factory.makeArchive(purpose=ArchivePurpose.PPA))
+        self.assertRaisesWithContent(
+            IllegalTarget,
+            "Package %s not published in %s"
+            % (dsp.sourcepackagename.name, dsp.distribution.displayname),
+            validate_target, task.bug, dsp)
+
+
+class TestValidateNewTarget(TestCaseWithFactory):
+
+    layer = DatabaseFunctionalLayer
+
+    def test_products_are_ok(self):
+        p1 = self.factory.makeProduct()
+        task = self.factory.makeBugTask(target=p1)
+        p2 = self.factory.makeProduct()
+        validate_new_target(task.bug, p2)
+
+    def test_calls_validate_target(self):
+        p = self.factory.makeProduct()
+        task = self.factory.makeBugTask(target=p)
+        self.assertRaisesWithContent(
+            IllegalTarget,
+            "A fix for this bug has already been requested for %s"
+            % p.displayname,
+            validate_new_target, task.bug, p)
+
+    def test_package_task_with_distribution_task_forbidden(self):
+        d = self.factory.makeDistribution()
+        dsp = self.factory.makeDistributionSourcePackage(distribution=d)
+        task = self.factory.makeBugTask(target=d)
+        self.assertRaisesWithContent(
+            IllegalTarget,
+            "This bug is already open on %s with no package specified. "
+            "You should fill in a package name for the existing bug."
+            % d.displayname,
+            validate_new_target, task.bug, dsp)
+
+    def test_distribution_task_with_package_task_forbidden(self):
+        d = self.factory.makeDistribution()
+        dsp = self.factory.makeDistributionSourcePackage(distribution=d)
+        task = self.factory.makeBugTask(target=dsp)
+        self.assertRaisesWithContent(
+            IllegalTarget,
+            "This bug is already on %s. Please specify an affected "
+            "package in which the bug has not yet been reported."
+            % d.displayname,
+            validate_new_target, task.bug, d)

=== modified file 'lib/lp/bugs/stories/bug-also-affects/xx-bug-also-affects.txt'
--- lib/lp/bugs/stories/bug-also-affects/xx-bug-also-affects.txt	2011-07-06 19:37:54 +0000
+++ lib/lp/bugs/stories/bug-also-affects/xx-bug-also-affects.txt	2011-07-25 23:06:42 +0000
@@ -76,7 +76,8 @@
     >>> browser.getControl("Save Changes").click()
     >>> print get_feedback_messages(browser.contents)
     [...There is 1 error in the data you entered...
-     u'This bug has already been reported on evolution (ubuntu).']
+     u'A fix for this bug has already been requested for evolution in
+       Ubuntu']
 
 Now let's add a Debian task to bug 1. Since Debian doesn't use Launchpad,
 we add a bug watch as well.
@@ -131,7 +132,8 @@
     'http://bugs.../ubuntu/+source/mozilla-firefox/+bug/1/+editstatus'
     >>> print get_feedback_messages(browser.contents)
     [...There is 1 error in the data you entered...
-     u'This bug has already been reported on alsa-utils (ubuntu).']
+     u'A fix for this bug has already been requested for alsa-utils in
+       Ubuntu']
 
     >>> browser.getControl('Package').value = 'pmount'
     >>> browser.getControl('Save Changes').click()