Merge lp:~cjwatson/launchpad/productseries-ubuntupkg-dsp-vocab into lp:launchpad

[Colin Watson]

Convert ProductSeries:+ubuntupkg to use the DistributionSourcePackage picker if the appropriate feature flag is set.

Convert ProductSeries:+ubuntupkg to use the DistributionSourcePackage picker if the appropriate feature flag is set.

I had to move most of BugTaskSourcePackageNameWidget into somewhat more common code to achieve this, since this view just asks for the source package name within Ubuntu rather than anything more generic. This will also be useful for the remaining views that need similar work after this (POTemplate:+edit, POTemplate:+admin, and TranslationImportQueueEntry:+index).

[William Grant (wgrant)]

review: Approve (code)

=== modified file 'lib/lp/app/widgets/popup.py'
--- lib/lp/app/widgets/popup.py	2016-07-21 20:22:00 +0000
+++ lib/lp/app/widgets/popup.py	2016-09-09 17:05:20 +0000
@@ -4,21 +4,38 @@
 """Single selection widget using a popup to select one item from many."""
 
 __metaclass__ = type
+__all__ = [
+    "BugTrackerPickerWidget",
+    "DistributionSourcePackagePickerWidget",
+    "PersonPickerWidget",
+    "SearchForUpstreamPopupWidget",
+    "SourcePackageNameWidgetBase",
+    "UbuntuSourcePackageNameWidget",
+    "VocabularyPickerWidget",
+    ]
 
 from lazr.restful.utils import safe_hasattr
 import simplejson
 from z3c.ptcompat import ViewPageTemplateFile
+from zope.component import getUtility
+from zope.formlib.interfaces import ConversionError
 from zope.formlib.itemswidgets import (
     ItemsWidgetBase,
     SingleDataHelper,
     )
-from zope.schema.interfaces import IChoice
+from zope.schema.interfaces import (
+    IChoice,
+    InvalidValue,
+    )
 
 from lp.app.browser.stringformatter import FormattersAPI
 from lp.app.browser.vocabulary import (
     get_person_picker_entry_metadata,
     vocabulary_filters,
     )
+from lp.app.errors import NotFoundError
+from lp.app.interfaces.launchpad import ILaunchpadCelebrities
+from lp.services.features import getFeatureFlag
 from lp.services.propertycache import cachedproperty
 from lp.services.webapp import canonical_url
 from lp.services.webapp.escaping import structured
@@ -288,3 +305,70 @@
         return self._prefix + 'distribution'
 
     distribution_name = ''
+
+
+class SourcePackageNameWidgetBase(DistributionSourcePackagePickerWidget):
+    """A base widget for choosing a SourcePackageName.
+
+    It accepts both binary and source package names.  Widgets inheriting
+    from this must implement `getDistribution`.
+
+    Most views should use LaunchpadTargetWidget or
+    DistributionSourcePackagePickerWidget instead, but this is useful in
+    cases where the distribution is fixed in some other way.
+    """
+
+    # Pages that use this widget don't display the distribution.
+    distribution_id = ''
+
+    def __init__(self, field, vocabulary, request):
+        super(SourcePackageNameWidgetBase, self).__init__(
+            field, vocabulary, request)
+        self.cached_values = {}
+
+    def getDistribution(self):
+        """Get the distribution used for package validation.
+
+        The package name has to be published in the returned distribution.
+        """
+        raise NotImplementedError
+
+    def _toFieldValue(self, input):
+        if not input:
+            return self.context.missing_value
+
+        distribution = self.getDistribution()
+        cached_value = self.cached_values.get(input)
+        if cached_value:
+            return cached_value
+        if bool(getFeatureFlag('disclosure.dsp_picker.enabled')):
+            try:
+                self.context.vocabulary.setDistribution(distribution)
+                return self.context.vocabulary.getTermByToken(input).value
+            except LookupError:
+                raise ConversionError(
+                    "Launchpad doesn't know of any source package named"
+                    " '%s' in %s." % (input, distribution.displayname))
+        # Else the untrusted SPN vocab was used so it needs secondary
+        # verification.
+        try:
+            source = distribution.guessPublishedSourcePackageName(input)
+        except NotFoundError:
+            try:
+                source = self.convertTokensToValues([input])[0]
+            except InvalidValue:
+                raise ConversionError(
+                    "Launchpad doesn't know of any source package named"
+                    " '%s' in %s." % (input, distribution.displayname))
+        self.cached_values[input] = source
+        return source
+
+
+class UbuntuSourcePackageNameWidget(SourcePackageNameWidgetBase):
+    """A widget to select Ubuntu packages."""
+
+    distribution_name = 'ubuntu'
+
+    def getDistribution(self):
+        """See `SourcePackageNameWidgetBase`"""
+        return getUtility(ILaunchpadCelebrities).ubuntu
 
=== modified file 'lib/lp/bugs/browser/bugtracker.py'
--- lib/lp/bugs/browser/bugtracker.py	2015-09-28 17:38:45 +0000
+++ lib/lp/bugs/browser/bugtracker.py	2016-09-09 17:05:20 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2013 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2016 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Bug tracker views."""
@@ -40,8 +40,8 @@
 from lp.app.interfaces.launchpad import ILaunchpadCelebrities
 from lp.app.validators import LaunchpadValidationError
 from lp.app.widgets.itemswidgets import LaunchpadRadioWidget
+from lp.app.widgets.popup import UbuntuSourcePackageNameWidget
 from lp.app.widgets.textwidgets import DelimitedListWidget
-from lp.bugs.browser.widgets.bugtask import UbuntuSourcePackageNameWidget
 from lp.bugs.interfaces.bugtracker import (
     BugTrackerType,
     IBugTracker,
 
=== modified file 'lib/lp/bugs/browser/widgets/bugtask.py'
--- lib/lp/bugs/browser/widgets/bugtask.py	2016-09-09 17:05:19 +0000
+++ lib/lp/bugs/browser/widgets/bugtask.py	2016-09-09 17:05:20 +0000
@@ -15,13 +15,11 @@
     "DBItemDisplayWidget",
     "FileBugSourcePackageNameWidget",
     "NewLineToSpacesWidget",
-    "UbuntuSourcePackageNameWidget",
     ]
 
 from z3c.ptcompat import ViewPageTemplateFile
 from zope.component import getUtility
 from zope.formlib.interfaces import (
-    ConversionError,
     IDisplayWidget,
     IInputWidget,
     InputErrors,
@@ -39,24 +37,17 @@
     implementer,
     Interface,
     )
-from zope.schema.interfaces import (
-    InvalidValue,
-    ValidationError,
-    )
+from zope.schema.interfaces import ValidationError
 from zope.schema.vocabulary import getVocabularyRegistry
 
 from lp import _
 from lp.app.browser.tales import TeamFormatterAPI
-from lp.app.errors import (
-    NotFoundError,
-    UnexpectedFormData,
-    )
-from lp.app.interfaces.launchpad import ILaunchpadCelebrities
+from lp.app.errors import UnexpectedFormData
 from lp.app.widgets.helpers import get_widget_template
 from lp.app.widgets.launchpadtarget import LaunchpadTargetWidget
 from lp.app.widgets.popup import (
-    DistributionSourcePackagePickerWidget,
     PersonPickerWidget,
+    SourcePackageNameWidgetBase,
     )
 from lp.app.widgets.textwidgets import (
     StrippedTextWidget,
@@ -483,27 +474,14 @@
         return vocabulary
 
 
-class BugTaskSourcePackageNameWidget(DistributionSourcePackagePickerWidget):
+class BugTaskSourcePackageNameWidget(SourcePackageNameWidgetBase):
     """A widget for associating a bugtask with a SourcePackageName.
 
     It accepts both binary and source package names.
     """
 
-    # Pages that use this widget don't display the distribution, but this
-    # can only be used by bugtasks on the distribution in question so the
-    # vocabulary will be able to work it out for itself.
-    distribution_id = ''
-
-    def __init__(self, field, vocabulary, request):
-        super(BugTaskSourcePackageNameWidget, self).__init__(
-            field, vocabulary, request)
-        self.cached_values = {}
-
     def getDistribution(self):
-        """Get the distribution used for package validation.
-
-        The package name has to be published in the returned distribution.
-        """
+        """See `SourcePackageNameWidgetBase`."""
         field = self.context
         distribution = field.context.distribution
         if distribution is None and field.context.distroseries is not None:
@@ -513,39 +491,8 @@
             " bugtasks on distributions or on distribution series.")
         return distribution
 
-    def _toFieldValue(self, input):
-        if not input:
-            return self.context.missing_value
-
-        distribution = self.getDistribution()
-        cached_value = self.cached_values.get(input)
-        if cached_value:
-            return cached_value
-        if bool(getFeatureFlag('disclosure.dsp_picker.enabled')):
-            try:
-                self.context.vocabulary.setDistribution(distribution)
-                return self.context.vocabulary.getTermByToken(input).value
-            except LookupError:
-                raise ConversionError(
-                    "Launchpad doesn't know of any source package named"
-                    " '%s' in %s." % (input, distribution.displayname))
-        # Else the untrusted SPN vocab was used so it needs secondary
-        # verification.
-        try:
-            source = distribution.guessPublishedSourcePackageName(input)
-        except NotFoundError:
-            try:
-                source = self.convertTokensToValues([input])[0]
-            except InvalidValue:
-                raise ConversionError(
-                    "Launchpad doesn't know of any source package named"
-                    " '%s' in %s." % (input, distribution.displayname))
-        self.cached_values[input] = source
-        return source
-
-
-class BugTaskAlsoAffectsSourcePackageNameWidget(
-    BugTaskSourcePackageNameWidget):
+
+class BugTaskAlsoAffectsSourcePackageNameWidget(SourcePackageNameWidgetBase):
     """Package widget for +distrotask.
 
     This widget works the same as `BugTaskSourcePackageNameWidget`, except
@@ -555,7 +502,7 @@
     distribution_id = 'field.distribution'
 
     def getDistribution(self):
-        """See `BugTaskSourcePackageNameWidget`."""
+        """See `SourcePackageNameWidgetBase`."""
         distribution_name = self.request.form.get('field.distribution')
         if distribution_name is None:
             raise UnexpectedFormData(
@@ -568,7 +515,7 @@
         return distribution
 
 
-class FileBugSourcePackageNameWidget(BugTaskSourcePackageNameWidget):
+class FileBugSourcePackageNameWidget(SourcePackageNameWidgetBase):
     """Package widget for +filebug.
 
     This widget works the same as `BugTaskSourcePackageNameWidget`, except
@@ -577,7 +524,7 @@
     """
 
     def getDistribution(self):
-        """See `BugTaskSourcePackageNameWidget`."""
+        """See `SourcePackageNameWidgetBase`."""
         field = self.context
         pillar = field.context.pillar
         assert IDistribution.providedBy(pillar), (
@@ -586,7 +533,7 @@
         return pillar
 
     def _toFieldValue(self, input):
-        """See `BugTaskSourcePackageNameWidget`."""
+        """See `SourcePackageNameWidgetBase`."""
         source = super(FileBugSourcePackageNameWidget, self)._toFieldValue(
             input)
         if (source is not None and
@@ -604,16 +551,6 @@
             return source
 
 
-class UbuntuSourcePackageNameWidget(BugTaskSourcePackageNameWidget):
-    """A widget to select Ubuntu packages."""
-
-    distribution_name = 'ubuntu'
-
-    def getDistribution(self):
-        """See `BugTaskSourcePackageNameWidget`"""
-        return getUtility(ILaunchpadCelebrities).ubuntu
-
-
 @implementer(IDisplayWidget)
 class AssigneeDisplayWidget(BrowserWidget):
     """A widget for displaying an assignee."""
 
=== modified file 'lib/lp/registry/browser/productseries.py'
--- lib/lp/registry/browser/productseries.py	2015-09-29 00:05:21 +0000
+++ lib/lp/registry/browser/productseries.py	2016-09-09 17:05:20 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2015 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2016 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """View classes for `IProductSeries`."""
@@ -28,6 +28,7 @@
 
 from operator import attrgetter
 
+from lazr.restful.interface import copy_field
 from z3c.ptcompat import ViewPageTemplateFile
 from zope.component import getUtility
 from zope.formlib import form
@@ -57,6 +58,7 @@
 from lp.app.enums import ServiceUsage
 from lp.app.errors import NotFoundError
 from lp.app.interfaces.launchpad import ILaunchpadCelebrities
+from lp.app.widgets.popup import UbuntuSourcePackageNameWidget
 from lp.app.widgets.textwidgets import StrippedTextWidget
 from lp.blueprints.browser.specificationtarget import (
     HasSpecificationsMenuMixin,
@@ -86,6 +88,9 @@
 from lp.registry.browser.product import ProductSetBranchView
 from lp.registry.enums import VCSType
 from lp.registry.errors import CannotPackageProprietaryProduct
+from lp.registry.interfaces.distributionsourcepackage import (
+    IDistributionSourcePackage,
+    )
 from lp.registry.interfaces.packaging import (
     IPackaging,
     IPackagingUtil,
@@ -93,6 +98,7 @@
 from lp.registry.interfaces.productseries import IProductSeries
 from lp.registry.interfaces.series import SeriesStatus
 from lp.services.config import config
+from lp.services.features import getFeatureFlag
 from lp.services.propertycache import cachedproperty
 from lp.services.webapp import (
     ApplicationMenu,
@@ -482,10 +488,25 @@
         return list(self.context.releases[:12])
 
 
+class IPackagingForm(IPackaging):
+
+    sourcepackagename = copy_field(
+        IPackaging['sourcepackagename'],
+        vocabularyName='DistributionSourcePackage')
+
+
 class ProductSeriesUbuntuPackagingView(LaunchpadFormView):
 
-    schema = IPackaging
+    @property
+    def schema(self):
+        """See `LaunchpadFormView`."""
+        if bool(getFeatureFlag('disclosure.dsp_picker.enabled')):
+            return IPackagingForm
+        else:
+            return IPackaging
+
     field_names = ['sourcepackagename', 'distroseries']
+    custom_widget('sourcepackagename', UbuntuSourcePackageNameWidget)
     page_title = 'Ubuntu source packaging'
     label = page_title
 
@@ -497,7 +518,11 @@
         self._ubuntu_series = self._ubuntu.currentseries
         try:
             package = self.context.getPackage(self._ubuntu_series)
-            self.default_sourcepackagename = package.sourcepackagename
+            if bool(getFeatureFlag('disclosure.dsp_picker.enabled')):
+                self.default_sourcepackagename = self._ubuntu.getSourcePackage(
+                    package.sourcepackagename)
+            else:
+                self.default_sourcepackagename = package.sourcepackagename
         except NotFoundError:
             # The package has never been set.
             self.default_sourcepackagename = None
@@ -555,6 +580,8 @@
     def validate(self, data):
         productseries = self.context
         sourcepackagename = data.get('sourcepackagename', None)
+        if IDistributionSourcePackage.providedBy(sourcepackagename):
+            sourcepackagename = sourcepackagename.sourcepackagename
         distroseries = self._getSubmittedSeries(data)
 
         packaging_util = getUtility(IPackagingUtil)
@@ -595,6 +622,8 @@
         # ubuntu series. if none exists, one will be created
         distroseries = self._getSubmittedSeries(data)
         sourcepackagename = data['sourcepackagename']
+        if IDistributionSourcePackage.providedBy(sourcepackagename):
+            sourcepackagename = sourcepackagename.sourcepackagename
         if getUtility(IPackagingUtil).packagingEntryExists(
             sourcepackagename, distroseries, productseries=self.context):
             # There is no change.
@@ -602,7 +631,7 @@
         try:
             self.context.setPackaging(
                 distroseries, sourcepackagename, self.user)
-        except CannotPackageProprietaryProduct, e:
+        except CannotPackageProprietaryProduct as e:
             self.request.response.addErrorNotification(str(e))
 
 
 
=== modified file 'lib/lp/registry/browser/tests/test_packaging.py'
--- lib/lp/registry/browser/tests/test_packaging.py	2016-06-17 15:46:57 +0000
+++ lib/lp/registry/browser/tests/test_packaging.py	2016-09-09 17:05:20 +0000
@@ -5,6 +5,10 @@
 
 __metaclass__ = type
 
+from testscenarios import (
+    load_tests_apply_scenarios,
+    WithScenarios,
+    )
 from zope.component import getUtility
 
 from lp.app.interfaces.launchpad import ILaunchpadCelebrities
@@ -15,6 +19,7 @@
     )
 from lp.registry.interfaces.product import IProductSet
 from lp.registry.interfaces.sourcepackagename import ISourcePackageNameSet
+from lp.services.features.testing import FeatureFixture
 from lp.testing import (
     login,
     logout,
@@ -28,13 +33,22 @@
 from lp.testing.views import create_initialized_view
 
 
-class TestProductSeriesUbuntuPackagingView(TestCaseWithFactory):
-    """Browser tests for deletion of Packaging objects."""
+class TestProductSeriesUbuntuPackagingView(WithScenarios, TestCaseWithFactory):
+    """Browser tests for adding Packaging objects."""
 
     layer = DatabaseFunctionalLayer
 
+    scenarios = [
+        ("spn_picker", {"features": {}}),
+        ("dsp_picker", {
+            "features": {u"disclosure.dsp_picker.enabled": u"on"},
+            }),
+        ]
+
     def setUp(self):
         super(TestProductSeriesUbuntuPackagingView, self).setUp()
+        if self.features:
+            self.useFixture(FeatureFixture(self.features))
         self.ubuntu = getUtility(ILaunchpadCelebrities).ubuntu
         self.hoary = self.ubuntu.getSeries('hoary')
         self.sourcepackagename = self.factory.makeSourcePackageName('hot')
@@ -88,7 +102,7 @@
              'hot</a> package in Hoary is already linked to another series.']
         self.assertEqual(view_errors, view.errors)
 
-    def test_sourcepackgename_required(self):
+    def test_sourcepackagename_required(self):
         # A source package name must be provided.
         form = {
             'field.distroseries': 'hoary',
@@ -188,3 +202,6 @@
             productseries=productseries,
             sourcepackagename=package_name,
             distroseries=distroseries))
+
+
+load_tests = load_tests_apply_scenarios
 
=== modified file 'lib/lp/registry/tests/test_distributionsourcepackage_vocabulary.py'
--- lib/lp/registry/tests/test_distributionsourcepackage_vocabulary.py	2016-09-09 17:05:19 +0000
+++ lib/lp/registry/tests/test_distributionsourcepackage_vocabulary.py	2016-09-09 17:05:20 +0000
@@ -175,6 +175,17 @@
         self.assertEqual(dsp.name, term.title)
         self.assertEqual(dsp, term.value)
 
+    def test_toTerm_dsp_no_distribution(self):
+        # The vocabulary can convert a DSP to a term even if it does not yet
+        # have a distribution.
+        dsp = self.factory.makeDistributionSourcePackage(
+            sourcepackagename='foo', with_db=False)
+        vocabulary = DistributionSourcePackageVocabulary(None)
+        term = vocabulary.toTerm(dsp)
+        self.assertEqual(dsp.name, term.token)
+        self.assertEqual(dsp.name, term.title)
+        self.assertEqual(dsp, term.value)
+
     def test_getTermByToken_error(self):
         # An error is raised if the token does not match a official DSP.
         distro = self.factory.makeDistribution()
 
=== modified file 'lib/lp/registry/vocabularies.py'
--- lib/lp/registry/vocabularies.py	2016-09-09 17:05:19 +0000
+++ lib/lp/registry/vocabularies.py	2016-09-09 17:05:20 +0000
@@ -2073,7 +2073,6 @@
 
     def toTerm(self, spn_or_dsp):
         """See `IVocabulary`."""
-        self._assertHasDistribution()
         dsp = None
         binary_names = None
         if isinstance(spn_or_dsp, tuple):
@@ -2088,9 +2087,10 @@
         if IDistributionSourcePackage.providedBy(spn_or_dsp):
             dsp = spn_or_dsp
         elif spn_or_dsp is not None:
+            self._assertHasDistribution()
             dsp = self.distribution.getSourcePackage(spn_or_dsp)
         if dsp is not None:
-            if dsp == self.dsp or dsp.is_official:
+            if dsp == self.dsp or dsp.is_official or self.distribution is None:
                 if binary_names:
                     # Search already did the hard work of looking up binary
                     # names.