launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #20957
[Merge] lp:~cjwatson/launchpad/productseries-ubuntupkg-dsp-vocab into lp:launchpad
Colin Watson has proposed merging lp:~cjwatson/launchpad/productseries-ubuntupkg-dsp-vocab into lp:launchpad with lp:~cjwatson/launchpad/distribution-filebug-dsp-vocab as a prerequisite.
Commit message:
Convert ProductSeries:+ubuntupkg to use the DistributionSourcePackage picker if the appropriate feature flag is set.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #42298 in Launchpad itself: "package picker lists unpublished (invalid) packages"
https://bugs.launchpad.net/launchpad/+bug/42298
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/productseries-ubuntupkg-dsp-vocab/+merge/305367
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).
--
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/productseries-ubuntupkg-dsp-vocab into lp:launchpad.
=== 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.
Follow ups
