launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #20958
[Merge] lp:~cjwatson/launchpad/potemplate-dsp-vocab into lp:launchpad
Colin Watson has proposed merging lp:~cjwatson/launchpad/potemplate-dsp-vocab into lp:launchpad with lp:~cjwatson/launchpad/productseries-ubuntupkg-dsp-vocab as a prerequisite.
Commit message:
Convert POTemplate:+edit and POTemplate:+admin 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/potemplate-dsp-vocab/+merge/305490
Convert POTemplate:+edit and POTemplate:+admin to use the DistributionSourcePackage picker if the appropriate feature flag is set.
We need another hack in the picker setup to cope with getting the distribution from the distroseries field, and we have to be a bit more careful about how we process empty forms so that GET requests work properly, but the rest is fairly straightforward.
--
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/potemplate-dsp-vocab into lp:launchpad.
=== modified file 'lib/lp/app/widgets/popup.py'
--- lib/lp/app/widgets/popup.py 2016-09-12 15:29:11 +0000
+++ lib/lp/app/widgets/popup.py 2016-09-12 15:29:12 +0000
@@ -305,6 +305,7 @@
return self._prefix + 'distribution'
distribution_name = ''
+ distroseries_id = ''
class SourcePackageNameWidgetBase(DistributionSourcePackagePickerWidget):
@@ -325,6 +326,13 @@
super(SourcePackageNameWidgetBase, self).__init__(
field, vocabulary, request)
self.cached_values = {}
+ if bool(getFeatureFlag('disclosure.dsp_picker.enabled')):
+ # The distribution may change later when we process form input,
+ # but setting it here makes it easier to construct some views,
+ # particularly edit views where we need to render the context.
+ distribution = self.getDistribution()
+ if distribution is not None:
+ self.context.vocabulary.setDistribution(distribution)
def getDistribution(self):
"""Get the distribution used for package validation.
=== modified file 'lib/lp/app/widgets/templates/distributionsourcepackage-picker.pt'
--- lib/lp/app/widgets/templates/distributionsourcepackage-picker.pt 2016-07-21 20:22:00 +0000
+++ lib/lp/app/widgets/templates/distributionsourcepackage-picker.pt 2016-09-12 15:29:12 +0000
@@ -9,6 +9,7 @@
var config = ${view/json_config};
var distribution_name = '${view/distribution_name}';
var distribution_id = '${view/distribution_id}';
+ var distroseries_id = '${view/distroseries_id}';
if (distribution_name !== '') {
config.getContextPath = function() {
return '/' + distribution_name;
@@ -17,6 +18,10 @@
config.getContextPath = function() {
return '/' + Y.DOM.byId(distribution_id).value;
};
+ } else if (distroseries_id !== '') {
+ config.getContextPath = function() {
+ return '/' + Y.DOM.byId(distroseries_id).value.split('/', 2)[0];
+ };
}
var show_widget_id = '${view/show_widget_id}';
Y.on('domready', function(e) {
=== modified file 'lib/lp/bugs/browser/widgets/bugtask.py'
--- lib/lp/bugs/browser/widgets/bugtask.py 2016-09-12 15:29:11 +0000
+++ lib/lp/bugs/browser/widgets/bugtask.py 2016-09-12 15:29:12 +0000
@@ -505,8 +505,7 @@
"""See `SourcePackageNameWidgetBase`."""
distribution_name = self.request.form.get('field.distribution')
if distribution_name is None:
- raise UnexpectedFormData(
- "field.distribution wasn't in the request")
+ return None
distribution = getUtility(IDistributionSet).getByName(
distribution_name)
if distribution is None:
=== modified file 'lib/lp/bugs/doc/bugtask-package-widget.txt'
--- lib/lp/bugs/doc/bugtask-package-widget.txt 2016-09-12 15:29:11 +0000
+++ lib/lp/bugs/doc/bugtask-package-widget.txt 2016-09-12 15:29:12 +0000
@@ -118,33 +118,30 @@
>>> request = LaunchpadTestRequest(
... form={'field.distribution': 'debian',
... 'field.sourcepackagename': 'linux-2.6.12'})
- >>> widget = BugTaskAlsoAffectsSourcePackageNameWidget(
- ... package_field, package_field.vocabulary, request)
- >>> widget.getDistribution().name
+ >>> BugTaskAlsoAffectsSourcePackageNameWidget(
+ ... package_field, package_field.vocabulary,
+ ... request).getDistribution().name
u'debian'
-+distrotask always supplies a valid distribution name. If the widget
-doesn't get a name, or the name isn't the name of a distro,
-UnexpectedFormData is raised.
++distrotask always supplies a valid distribution name or none at all. If the
+name isn't the name of a distro, UnexpectedFormData is raised.
>>> request = LaunchpadTestRequest(
... form={'field.distribution': 'non-existing',
... 'field.sourcepackagename': 'linux-2.6.12'})
- >>> widget = BugTaskAlsoAffectsSourcePackageNameWidget(
- ... package_field, package_field.vocabulary, request)
- >>> widget.getDistribution().name
- Traceback (most recent call last):
- ...
- UnexpectedFormData: ...
-
- >>> request = LaunchpadTestRequest(
- ... form={'field.sourcepackagename': 'linux-2.6.12'})
- >>> widget = BugTaskAlsoAffectsSourcePackageNameWidget(
- ... package_field, package_field.vocabulary, request)
- >>> widget.getDistribution().name
- Traceback (most recent call last):
- ...
- UnexpectedFormData: ...
+ >>> BugTaskAlsoAffectsSourcePackageNameWidget(
+ ... package_field, package_field.vocabulary,
+ ... request).getDistribution().name
+ Traceback (most recent call last):
+ ...
+ UnexpectedFormData: ...
+
+A GET request usually won't supply a distribution name at all.
+
+ >>> request = LaunchpadTestRequest(form={})
+ >>> BugTaskAlsoAffectsSourcePackageNameWidget(
+ ... package_field, package_field.vocabulary,
+ ... request).getDistribution()
FileBugSourcePackageNameWidget
=== modified file 'lib/lp/translations/browser/potemplate.py'
--- lib/lp/translations/browser/potemplate.py 2016-06-09 20:13:17 +0000
+++ lib/lp/translations/browser/potemplate.py 2016-09-12 15:29:12 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2012 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).
"""Browser code for PO templates."""
@@ -26,6 +26,7 @@
import operator
import os.path
+from lazr.restful.interface import copy_field
from lazr.restful.utils import smartquote
import pytz
from storm.expr import (
@@ -41,6 +42,7 @@
from lp import _
from lp.app.browser.launchpadform import (
action,
+ custom_widget,
LaunchpadEditFormView,
ReturnToReferrerMixin,
)
@@ -51,11 +53,15 @@
)
from lp.app.errors import NotFoundError
from lp.app.validators.name import valid_name
+from lp.registry.interfaces.distributionsourcepackage import (
+ IDistributionSourcePackage,
+ )
from lp.registry.interfaces.role import IPersonRoles
from lp.registry.model.packaging import Packaging
from lp.registry.model.product import Product
from lp.registry.model.productseries import ProductSeries
from lp.registry.model.sourcepackagename import SourcePackageName
+from lp.services.features import getFeatureFlag
from lp.services.helpers import is_tar_filename
from lp.services.propertycache import cachedproperty
from lp.services.webapp import (
@@ -86,6 +92,10 @@
from lp.translations.browser.translationsharing import (
TranslationSharingDetailsMixin,
)
+from lp.translations.browser.widgets.potemplate import (
+ POTemplateAdminSourcePackageNameWidget,
+ POTemplateEditSourcePackageNameWidget,
+ )
from lp.translations.interfaces.pofile import IPOFileSet
from lp.translations.interfaces.potemplate import (
IPOTemplate,
@@ -492,10 +502,29 @@
return POTemplateView.pofiles(self, preferred_only=True)
+class IPOTemplateEditForm(IPOTemplate):
+
+ sourcepackagename = copy_field(
+ IPOTemplate['sourcepackagename'],
+ vocabularyName='DistributionSourcePackage')
+
+ from_sourcepackagename = copy_field(
+ IPOTemplate['from_sourcepackagename'],
+ vocabularyName='DistributionSourcePackage')
+
+
class POTemplateEditView(ReturnToReferrerMixin, LaunchpadEditFormView):
"""View class that lets you edit a POTemplate object."""
- schema = IPOTemplate
+ @property
+ def schema(self):
+ """See `LaunchpadFormView`."""
+ if bool(getFeatureFlag('disclosure.dsp_picker.enabled')):
+ return IPOTemplateEditForm
+ else:
+ return IPOTemplate
+
+ custom_widget('sourcepackagename', POTemplateEditSourcePackageNameWidget)
label = 'Edit translation template details'
page_title = 'Edit details'
PRIORITY_MIN_VALUE = 0
@@ -516,6 +545,14 @@
return field_names
@property
+ def adapters(self):
+ """See `LaunchpadFormView`."""
+ if bool(getFeatureFlag('disclosure.dsp_picker.enabled')):
+ return {IPOTemplateEditForm: self.context}
+ else:
+ return {}
+
+ @property
def _return_url(self):
# We override the ReturnToReferrerMixin _return_url because it might
# change when any of the productseries, distroseries,
@@ -548,6 +585,9 @@
context.setActive(iscurrent)
old_description = context.description
old_translation_domain = context.translation_domain
+ for field in ('sourcepackagename', 'from_sourcepackagename'):
+ if IDistributionSourcePackage.providedBy(data.get(field)):
+ data[field] = data[field].sourcepackagename
self.updateContextFromData(data)
if old_description != context.description:
self.user.assignKarma(
@@ -565,6 +605,8 @@
"""Return a POTemplateSubset corresponding to the chosen target."""
sourcepackagename = data.get('sourcepackagename',
self.context.sourcepackagename)
+ if IDistributionSourcePackage.providedBy(sourcepackagename):
+ sourcepackagename = sourcepackagename.sourcepackagename
return getUtility(IPOTemplateSet).getSubset(
distroseries=self.context.distroseries,
sourcepackagename=sourcepackagename,
@@ -582,6 +624,8 @@
distroseries = data.get('distroseries', self.context.distroseries)
sourcepackagename = data.get(
'sourcepackagename', self.context.sourcepackagename)
+ if IDistributionSourcePackage.providedBy(sourcepackagename):
+ sourcepackagename = sourcepackagename.sourcepackagename
productseries = data.get('productseries', None)
sourcepackage_changed = (
distroseries is not None and
@@ -656,6 +700,9 @@
'from_sourcepackagename', 'sourcepackageversion',
'languagepack', 'path', 'source_file_format', 'priority',
'date_last_updated']
+ custom_widget('sourcepackagename', POTemplateAdminSourcePackageNameWidget)
+ custom_widget(
+ 'from_sourcepackagename', POTemplateAdminSourcePackageNameWidget)
label = 'Administer translation template'
page_title = "Administer"
@@ -663,6 +710,8 @@
"""Return a POTemplateSubset corresponding to the chosen target."""
distroseries = data.get('distroseries')
sourcepackagename = data.get('sourcepackagename')
+ if IDistributionSourcePackage.providedBy(sourcepackagename):
+ sourcepackagename = sourcepackagename.sourcepackagename
productseries = data.get('productseries')
if distroseries is not None and productseries is not None:
=== modified file 'lib/lp/translations/browser/tests/test_potemplate_views.py'
--- lib/lp/translations/browser/tests/test_potemplate_views.py 2012-12-10 13:43:47 +0000
+++ lib/lp/translations/browser/tests/test_potemplate_views.py 2016-09-12 15:29:12 +0000
@@ -1,25 +1,46 @@
-# Copyright 2011 Canonical Ltd. This software is licensed under the
+# Copyright 2011-2016 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Module doc."""
__metaclass__ = type
+from testscenarios import (
+ load_tests_apply_scenarios,
+ WithScenarios,
+ )
+from lp.services.features.testing import FeatureFixture
from lp.services.webapp.escaping import html_escape
from lp.services.webapp.servers import LaunchpadTestRequest
-from lp.testing import TestCaseWithFactory
+from lp.testing import (
+ celebrity_logged_in,
+ TestCaseWithFactory,
+ )
from lp.testing.layers import DatabaseFunctionalLayer
+from lp.testing.views import create_initialized_view
from lp.translations.browser.potemplate import (
POTemplateAdminView,
POTemplateEditView,
)
-class TestPOTemplateEditViewValidation(TestCaseWithFactory):
+class TestPOTemplateEditViewValidation(WithScenarios, TestCaseWithFactory):
layer = DatabaseFunctionalLayer
+ scenarios = [
+ ("spn_picker", {"features": {}}),
+ ("dsp_picker", {
+ "features": {u"disclosure.dsp_picker.enabled": u"on"},
+ }),
+ ]
+
+ def setUp(self):
+ super(TestPOTemplateEditViewValidation, self).setUp()
+ if self.features:
+ self.useFixture(FeatureFixture(self.features))
+
def _makeData(self, potemplate, **kwargs):
"""Create form data for the given template with some changed values.
@@ -131,6 +152,23 @@
[u'Source package already has a template with that same domain.'],
view.errors)
+ def test_change_sourcepackage(self):
+ # Changing the source package is honoured.
+ distroseries = self.factory.makeDistroSeries()
+ potemplate = self.factory.makePOTemplate(distroseries=distroseries)
+ dsp = self.factory.makeDSPCache(distroseries=distroseries)
+ form = {
+ 'field.name': potemplate.name,
+ 'field.distroseries': distroseries.name,
+ 'field.sourcepackagename': dsp.sourcepackagename.name,
+ 'field.actions.change': 'Change',
+ }
+ with celebrity_logged_in('rosetta_experts'):
+ view = create_initialized_view(potemplate, '+edit', form=form)
+ self.assertEqual([], view.errors)
+ self.assertEqual(
+ dsp.sourcepackagename.name, potemplate.sourcepackagename.name)
+
class TestPOTemplateAdminViewValidation(TestPOTemplateEditViewValidation):
@@ -194,3 +232,30 @@
self.assertEqual(
[u'Choose a distribution release series or a project '
u'release series, but not both.'], view.errors)
+
+ def test_change_from_sourcepackage(self):
+ # Changing the source package the template comes from is honoured.
+ distroseries = self.factory.makeDistroSeries()
+ dsp = self.factory.makeDSPCache(distroseries=distroseries)
+ potemplate = self.factory.makePOTemplate(
+ distroseries=distroseries, sourcepackagename=dsp.sourcepackagename)
+ from_dsp = self.factory.makeDSPCache(distroseries=distroseries)
+ form = {
+ 'field.name': potemplate.name,
+ 'field.distroseries': '%s/%s' % (
+ distroseries.distribution.name, distroseries.name),
+ 'field.sourcepackagename': dsp.sourcepackagename.name,
+ 'field.from_sourcepackagename': from_dsp.sourcepackagename.name,
+ 'field.actions.change': 'Change',
+ }
+ with celebrity_logged_in('rosetta_experts'):
+ view = create_initialized_view(potemplate, '+admin', form=form)
+ self.assertEqual([], view.errors)
+ self.assertEqual(
+ dsp.sourcepackagename.name, potemplate.sourcepackagename.name)
+ self.assertEqual(
+ from_dsp.sourcepackagename.name,
+ potemplate.from_sourcepackagename.name)
+
+
+load_tests = load_tests_apply_scenarios
=== added directory 'lib/lp/translations/browser/widgets'
=== added file 'lib/lp/translations/browser/widgets/__init__.py'
=== added file 'lib/lp/translations/browser/widgets/potemplate.py'
--- lib/lp/translations/browser/widgets/potemplate.py 1970-01-01 00:00:00 +0000
+++ lib/lp/translations/browser/widgets/potemplate.py 2016-09-12 15:29:12 +0000
@@ -0,0 +1,62 @@
+# Copyright 2016 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Widgets related to `IPOTemplate`."""
+
+from __future__ import absolute_import, print_function, unicode_literals
+
+__metaclass__ = type
+__all__ = [
+ "POTemplateAdminSourcePackageNameWidget",
+ "POTemplateEditSourcePackageNameWidget",
+ ]
+
+from lp.app.errors import UnexpectedFormData
+from lp.app.widgets.popup import SourcePackageNameWidgetBase
+from lp.registry.vocabularies import DistroSeriesVocabulary
+
+
+class POTemplateEditSourcePackageNameWidget(SourcePackageNameWidgetBase):
+ """A widget for associating a POTemplate with a SourcePackageName.
+
+ This is suitable for use on POTemplate:+edit, where the distribution is
+ fixed.
+ """
+
+ @property
+ def distribution_name(self):
+ distribution = self.getDistribution()
+ if distribution is not None:
+ return distribution.name
+ else:
+ return ''
+
+ def getDistribution(self):
+ """See `SourcePackageNameWidgetBase`."""
+ return self.context.context.distribution
+
+
+class POTemplateAdminSourcePackageNameWidget(SourcePackageNameWidgetBase):
+ """A widget for associating a POTemplate with a SourcePackageName.
+
+ This is suitable for use on POTemplate:+admin, where the distribution
+ may be changed via the distroseries field.
+ """
+
+ @property
+ def distroseries_id(self):
+ return self._prefix + 'distroseries'
+
+ def getDistribution(self):
+ """See `SourcePackageNameWidgetBase`."""
+ distroseries_token = self.request.form.get('field.distroseries')
+ if distroseries_token is None:
+ # Fall back to the POTemplate's current distribution.
+ return self.context.context.distribution
+ distroseries_vocab = DistroSeriesVocabulary()
+ try:
+ term = distroseries_vocab.getTermByToken(distroseries_token)
+ except LookupError:
+ raise UnexpectedFormData(
+ "No such distribution series: %s" % distroseries_token)
+ return term.value.distribution
=== added directory 'lib/lp/translations/browser/widgets/tests'
=== added file 'lib/lp/translations/browser/widgets/tests/__init__.py'
=== added file 'lib/lp/translations/browser/widgets/tests/test_potemplate.py'
--- lib/lp/translations/browser/widgets/tests/test_potemplate.py 1970-01-01 00:00:00 +0000
+++ lib/lp/translations/browser/widgets/tests/test_potemplate.py 2016-09-12 15:29:12 +0000
@@ -0,0 +1,134 @@
+# Copyright 2016 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Test the POTemplate widgets."""
+
+from __future__ import absolute_import, print_function, unicode_literals
+
+__metaclass__ = type
+
+from testscenarios import (
+ load_tests_apply_scenarios,
+ WithScenarios,
+ )
+
+from lp.app.errors import UnexpectedFormData
+from lp.services.features.testing import FeatureFixture
+from lp.services.webapp.servers import LaunchpadTestRequest
+from lp.testing import TestCaseWithFactory
+from lp.testing.layers import DatabaseFunctionalLayer
+from lp.translations.browser.potemplate import IPOTemplateEditForm
+from lp.translations.browser.widgets.potemplate import (
+ POTemplateAdminSourcePackageNameWidget,
+ POTemplateEditSourcePackageNameWidget,
+ )
+from lp.translations.interfaces.potemplate import IPOTemplate
+
+
+class TestPOTemplateEditSourcePackageNameWidget(
+ WithScenarios, TestCaseWithFactory):
+
+ layer = DatabaseFunctionalLayer
+
+ scenarios = [
+ ("spn_picker", {
+ "features": {},
+ "interface": IPOTemplate,
+ }),
+ ("dsp_picker", {
+ "features": {u"disclosure.dsp_picker.enabled": u"on"},
+ "interface": IPOTemplateEditForm,
+ }),
+ ]
+
+ def setUp(self):
+ super(TestPOTemplateEditSourcePackageNameWidget, self).setUp()
+ if self.features:
+ self.useFixture(FeatureFixture(self.features))
+
+ def makeWidget(self, potemplate, form=None):
+ field = self.interface["sourcepackagename"]
+ bound_field = field.bind(potemplate)
+ request = LaunchpadTestRequest(form=form)
+ return POTemplateEditSourcePackageNameWidget(
+ bound_field, bound_field.vocabulary, request)
+
+ def test_productseries(self):
+ potemplate = self.factory.makePOTemplate()
+ widget = self.makeWidget(potemplate)
+ self.assertIsNone(widget.getDistribution())
+ self.assertEqual("", widget.distribution_name)
+
+ def test_distroseries(self):
+ distroseries = self.factory.makeDistroSeries()
+ potemplate = self.factory.makePOTemplate(distroseries=distroseries)
+ widget = self.makeWidget(potemplate)
+ self.assertEqual(distroseries.distribution, widget.getDistribution())
+ self.assertEqual(
+ distroseries.distribution.name, widget.distribution_name)
+
+
+class TestPOTemplateAdminSourcePackageNameWidget(
+ WithScenarios, TestCaseWithFactory):
+
+ layer = DatabaseFunctionalLayer
+
+ scenarios = [
+ ("spn_picker", {
+ "features": {},
+ "interface": IPOTemplate,
+ }),
+ ("dsp_picker", {
+ "features": {u"disclosure.dsp_picker.enabled": u"on"},
+ "interface": IPOTemplateEditForm,
+ }),
+ ]
+
+ def setUp(self):
+ super(TestPOTemplateAdminSourcePackageNameWidget, self).setUp()
+ if self.features:
+ self.useFixture(FeatureFixture(self.features))
+
+ def makeWidget(self, potemplate, form=None):
+ field = self.interface["sourcepackagename"]
+ bound_field = field.bind(potemplate)
+ request = LaunchpadTestRequest(form=form)
+ return POTemplateAdminSourcePackageNameWidget(
+ bound_field, bound_field.vocabulary, request)
+
+ def test_distroseries_id(self):
+ potemplate = self.factory.makePOTemplate()
+ distroseries = self.factory.makeDistroSeries()
+ form = {
+ "field.distroseries": "%s/%s" % (
+ distroseries.distribution.name, distroseries.name),
+ }
+ widget = self.makeWidget(potemplate, form=form)
+ self.assertEqual("field.distroseries", widget.distroseries_id)
+
+ def test_getDistribution(self):
+ potemplate = self.factory.makePOTemplate()
+ distroseries = self.factory.makeDistroSeries()
+ form = {
+ "field.distroseries": "%s/%s" % (
+ distroseries.distribution.name, distroseries.name),
+ }
+ widget = self.makeWidget(potemplate, form=form)
+ self.assertEqual(distroseries.distribution, widget.getDistribution())
+
+ def test_getDistribution_missing_field(self):
+ distroseries = self.factory.makeDistroSeries()
+ potemplate = self.factory.makePOTemplate(distroseries=distroseries)
+ widget = self.makeWidget(potemplate, form={})
+ self.assertEqual(distroseries.distribution, widget.getDistribution())
+
+ def test_getDistribution_non_existent_distroseries(self):
+ potemplate = self.factory.makePOTemplate()
+ form = {"field.distroseries": "not-a-distribution/not-a-series"}
+ self.assertRaises(
+ UnexpectedFormData,
+ lambda: (
+ self.makeWidget(potemplate, form=form).getDistribution().name))
+
+
+load_tests = load_tests_apply_scenarios
Follow ups
