launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #20836
[Merge] lp:~cjwatson/launchpad/fix-dsp-vocab-picker into lp:launchpad
Colin Watson has proposed merging lp:~cjwatson/launchpad/fix-dsp-vocab-picker into lp:launchpad.
Commit message:
Customise the picker for DistributionSourcePackageVocabulary to make vocabulary requests in the context of the currently-selected distribution.
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/fix-dsp-vocab-picker/+merge/300782
Customise the picker for DistributionSourcePackageVocabulary to make vocabulary requests in the context of the currently-selected distribution. Otherwise the vocabulary only works if explicitly given the <distro>/<package> search format, which isn't very user-friendly.
--
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/fix-dsp-vocab-picker into lp:launchpad.
=== modified file 'lib/lp/app/javascript/picker/picker_patcher.js'
--- lib/lp/app/javascript/picker/picker_patcher.js 2012-09-07 14:42:48 +0000
+++ lib/lp/app/javascript/picker/picker_patcher.js 2016-07-21 15:25:29 +0000
@@ -1,4 +1,4 @@
-/* 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).
*/
@@ -620,7 +620,9 @@
// use the context to limit the results to the same project.
var uri = '';
- if (Y.Lang.isValue(config.context)){
+ if (Y.Lang.isFunction(config.getContextPath)) {
+ uri += config.getContextPath() + '/';
+ } else if (Y.Lang.isValue(config.context)) {
uri += Y.lp.get_url_path(
config.context.get('web_link')) + '/';
}
=== modified file 'lib/lp/app/javascript/picker/tests/test_picker_patcher.js'
--- lib/lp/app/javascript/picker/tests/test_picker_patcher.js 2013-03-20 03:41:40 +0000
+++ lib/lp/app/javascript/picker/tests/test_picker_patcher.js 2016-07-21 15:25:29 +0000
@@ -1,4 +1,4 @@
-/* Copyright 2012 Canonical Ltd. This software is licensed under the
+/* Copyright 2012-2016 Canonical Ltd. This software is licensed under the
* GNU Affero General Public License version 3 (see the file LICENSE). */
YUI.add('lp.app.picker.test', function (Y) {
@@ -33,7 +33,7 @@
}
tests.suite.add(new Y.Test.Case({
- name: 'picker_yesyno_validation',
+ name: 'picker_yesno_validation',
setUp: function() {
this.vocabulary = [
@@ -514,6 +514,39 @@
tests.suite.add(new Y.Test.Case({
+ name: 'picker_custom_context_path',
+
+ create_picker: function(yio, extra_config) {
+ var config = {yio: yio};
+ if (extra_config !== undefined) {
+ config = Y.merge(extra_config, config);
+ }
+ return Y.lp.app.picker.addPickerPatcher(
+ "Foo",
+ "foo/bar",
+ "test_link",
+ "picker_id",
+ config);
+ },
+
+ test_custom_context_path_honoured: function() {
+ var mock_io = new Y.lp.testing.mockio.MockIo();
+ var extra_config = {
+ "getContextPath": function() { return "/gentoo"; }
+ };
+ var picker = this.create_picker(mock_io, extra_config);
+ picker.fire('search', 'package');
+ Assert.areEqual(1, mock_io.requests.length);
+ Assert.areEqual(
+ '/gentoo/@@+huge-vocabulary?name=Foo&search_text=package&' +
+ 'batch=6&start=0', mock_io.requests[0].url);
+ cleanup_widget(picker);
+ }
+
+ }));
+
+ tests.suite.add(new Y.Test.Case({
+
name: 'picker_error_handling',
setUp: function() {
=== modified file 'lib/lp/app/widgets/launchpadtarget.py'
--- lib/lp/app/widgets/launchpadtarget.py 2016-06-10 22:06:13 +0000
+++ lib/lp/app/widgets/launchpadtarget.py 2016-07-21 15:25:29 +0000
@@ -29,6 +29,7 @@
from lp.app.interfaces.launchpad import ILaunchpadCelebrities
from lp.app.validators import LaunchpadValidationError
from lp.app.widgets.itemswidgets import LaunchpadDropdownWidget
+from lp.app.widgets.popup import VocabularyPickerWidget
from lp.registry.interfaces.distribution import IDistribution
from lp.registry.interfaces.distributionsourcepackage import (
IDistributionSourcePackage,
@@ -194,3 +195,13 @@
self.setUpSubWidgets()
self.setUpOptions()
return self.template()
+
+
+class LaunchpadTargetPopupWidget(VocabularyPickerWidget):
+ """Custom popup widget for choosing distribution/package combinations."""
+
+ __call__ = ViewPageTemplateFile('templates/launchpad-target-picker.pt')
+
+ @property
+ def distribution_id(self):
+ return self._prefix + "distribution"
=== modified file 'lib/lp/app/widgets/templates/form-picker-macros.pt'
--- lib/lp/app/widgets/templates/form-picker-macros.pt 2012-02-01 23:47:54 +0000
+++ lib/lp/app/widgets/templates/form-picker-macros.pt 2016-07-21 15:25:29 +0000
@@ -29,7 +29,7 @@
</tal:search_results>
<tal:chooseLink replace="structure view/chooseLink" />
- <script tal:content="structure string:
+ <script metal:define-slot="add-picker" tal:content="structure string:
LPJS.use('node', 'lp.app.picker', function(Y) {
var config = ${view/json_config};
var show_widget_id = '${view/show_widget_id}';
=== added file 'lib/lp/app/widgets/templates/launchpad-target-picker.pt'
--- lib/lp/app/widgets/templates/launchpad-target-picker.pt 1970-01-01 00:00:00 +0000
+++ lib/lp/app/widgets/templates/launchpad-target-picker.pt 2016-07-21 15:25:29 +0000
@@ -0,0 +1,23 @@
+<tal:root
+ xmlns:tal="http://xml.zope.org/namespaces/tal";
+ xmlns:metal="http://xml.zope.org/namespaces/metal";
+ omit-tag="">
+
+<metal:form-picker use-macro="context/@@form-picker-macros/form-picker">
+ <script metal:fill-slot="add-picker" tal:content="structure string:
+ LPJS.use('node', 'lp.app.picker', function(Y) {
+ var config = ${view/json_config};
+ if ('${view/distribution_id|nothing}' !== '') {
+ config.getContextPath = function() {
+ return '/' + Y.DOM.byId('${view/distribution_id|nothing}').value;
+ };
+ }
+ var show_widget_id = '${view/show_widget_id}';
+ Y.on('domready', function(e) {
+ Y.lp.app.picker.addPicker(config, show_widget_id);
+ });
+ });
+ "/>
+</metal:form-picker>
+
+</tal:root>
=== modified file 'lib/lp/app/widgets/tests/test_launchpadtarget.py'
--- lib/lp/app/widgets/tests/test_launchpadtarget.py 2016-07-11 22:35:59 +0000
+++ lib/lp/app/widgets/tests/test_launchpadtarget.py 2016-07-21 15:25:29 +0000
@@ -6,7 +6,10 @@
import re
from BeautifulSoup import BeautifulSoup
-from lazr.restful.fields import Reference
+from lazr.restful.fields import (
+ Reference,
+ ReferenceChoice,
+ )
from zope.formlib.interfaces import (
IBrowserWidget,
IInputWidget,
@@ -16,9 +19,13 @@
implementer,
Interface,
)
+from zope.schema.vocabulary import getVocabularyRegistry
from lp.app.validators import LaunchpadValidationError
-from lp.app.widgets.launchpadtarget import LaunchpadTargetWidget
+from lp.app.widgets.launchpadtarget import (
+ LaunchpadTargetPopupWidget,
+ LaunchpadTargetWidget,
+ )
from lp.registry.vocabularies import (
DistributionSourcePackageVocabulary,
DistributionVocabulary,
@@ -82,7 +89,7 @@
# The render template is setup.
self.assertTrue(
self.widget.template.filename.endswith('launchpad-target.pt'),
- 'Template was not setup.')
+ 'Template was not set up.')
def test_default_option(self):
# This package field is the default option.
@@ -329,3 +336,33 @@
fields = soup.findAll(['input', 'select'], {'id': re.compile('.*')})
ids = [field['id'] for field in fields]
self.assertContentEqual(expected_ids, ids)
+
+
+class LaunchpadTargetPopupWidgetTestCase(TestCaseWithFactory):
+ """Test the LaunchpadTargetPopupWidget class."""
+
+ layer = DatabaseFunctionalLayer
+
+ def setUp(self):
+ super(LaunchpadTargetPopupWidgetTestCase, self).setUp()
+ context = Thing()
+ field = ReferenceChoice(
+ __name__='target', schema=Interface, title=u'target',
+ vocabulary='DistributionSourcePackage')
+ field = field.bind(context)
+ vocabulary = getVocabularyRegistry().get(
+ context, 'DistributionSourcePackage')
+ request = LaunchpadTestRequest()
+ self.widget = LaunchpadTargetPopupWidget(field, vocabulary, request)
+ self.widget.setPrefix('field.target')
+
+ def test_distribution_id(self):
+ self.assertEqual(
+ 'field.target.distribution', self.widget.distribution_id)
+
+ def test_call(self):
+ markup = self.widget()
+ # It's difficult to do a particularly accurate test here, but we can
+ # at least check that code to get the value of the distribution
+ # field is present.
+ self.assertIn("Y.DOM.byId('field.target.distribution').value", markup)
=== modified file 'lib/lp/services/webapp/configure.zcml'
--- lib/lp/services/webapp/configure.zcml 2014-11-28 22:07:05 +0000
+++ lib/lp/services/webapp/configure.zcml 2016-07-21 15:25:29 +0000
@@ -1,4 +1,4 @@
-<!-- Copyright 2009-2011 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).
-->
@@ -413,6 +413,17 @@
permission="zope.Public"
/>
+ <!-- Define the widget used by fields that use
+ DistributionSourcePackageVocabulary. -->
+ <view
+ type="zope.publisher.interfaces.browser.IBrowserRequest"
+ for="zope.schema.interfaces.IChoice
+ lp.registry.vocabularies.DistributionSourcePackageVocabulary"
+ provides="zope.formlib.interfaces.IInputWidget"
+ factory="lp.app.widgets.launchpadtarget.LaunchpadTargetPopupWidget"
+ permission="zope.Public"
+ />
+
<!-- A simple view used by the page tests. -->
<browser:page
for="lp.services.webapp.interfaces.ILaunchpadRoot"
Follow ups
