Merge lp:~cjwatson/launchpad/potemplate-dsp-vocab into lp:launchpad

[Colin Watson]

Convert POTemplate:+edit and POTemplate:+admin to use the DistributionSourcePackage picker if the appropriate feature flag is set.

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.

[William Grant (wgrant)]

review: Approve (code)

=== 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