Merge lp:~cjwatson/launchpad/fix-dsp-vocab-picker into lp:launchpad

Proposed by Colin Watson on 2016-07-21
Status: Merged
Merged at revision: 18164
Proposed branch: lp:~cjwatson/launchpad/fix-dsp-vocab-picker
Merge into: lp:launchpad
Diff against target: 781 lines (+216/-173)
12 files modified
lib/lp/app/javascript/picker/picker_patcher.js (+4/-2)
lib/lp/app/javascript/picker/tests/test_picker_patcher.js (+35/-2)
lib/lp/app/widgets/popup.py (+14/-1)
lib/lp/app/widgets/templates/distributionsourcepackage-picker.pt (+29/-0)
lib/lp/app/widgets/templates/form-picker-macros.pt (+1/-1)
lib/lp/app/widgets/tests/test_launchpadtarget.py (+2/-12)
lib/lp/app/widgets/tests/test_popup.py (+33/-7)
lib/lp/bugs/browser/tests/test_bugalsoaffects.py (+1/-18)
lib/lp/bugs/browser/widgets/bugtask.py (+11/-2)
lib/lp/registry/tests/test_distributionsourcepackage_vocabulary.py (+45/-92)
lib/lp/registry/vocabularies.py (+29/-35)
lib/lp/services/webapp/configure.zcml (+12/-1)
To merge this branch: bzr merge lp:~cjwatson/launchpad/fix-dsp-vocab-picker
Reviewer Review Type Date Requested Status
William Grant code 2016-07-21 Approve on 2016-07-26
Review via email: mp+300782@code.launchpad.net

Commit Message

Customise the picker for DistributionSourcePackageVocabulary to make vocabulary requests in the context of the currently-selected distribution.

Description of the Change

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.

I've also changed the vocabulary itself to return just the package name as the term's token and title, rather than distribution/package. This is less strictly correct, but given that it's always being fed into a package name text box it works a lot better in practice (we don't want that text box to receive e.g. "ubuntu/firefox"), and since the vocabulary always has its distribution context set from somewhere else it works fine in practice.

To post a comment you must log in.
William Grant (wgrant) :
review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/app/javascript/picker/picker_patcher.js'
2--- lib/lp/app/javascript/picker/picker_patcher.js 2012-09-07 14:42:48 +0000
3+++ lib/lp/app/javascript/picker/picker_patcher.js 2016-07-27 17:22:45 +0000
4@@ -1,4 +1,4 @@
5-/* Copyright 2011 Canonical Ltd. This software is licensed under the
6+/* Copyright 2011-2016 Canonical Ltd. This software is licensed under the
7 * GNU Affero General Public License version 3 (see the file LICENSE).
8 */
9
10@@ -620,7 +620,9 @@
11 // use the context to limit the results to the same project.
12
13 var uri = '';
14- if (Y.Lang.isValue(config.context)){
15+ if (Y.Lang.isFunction(config.getContextPath)) {
16+ uri += config.getContextPath() + '/';
17+ } else if (Y.Lang.isValue(config.context)) {
18 uri += Y.lp.get_url_path(
19 config.context.get('web_link')) + '/';
20 }
21
22=== modified file 'lib/lp/app/javascript/picker/tests/test_picker_patcher.js'
23--- lib/lp/app/javascript/picker/tests/test_picker_patcher.js 2013-03-20 03:41:40 +0000
24+++ lib/lp/app/javascript/picker/tests/test_picker_patcher.js 2016-07-27 17:22:45 +0000
25@@ -1,4 +1,4 @@
26-/* Copyright 2012 Canonical Ltd. This software is licensed under the
27+/* Copyright 2012-2016 Canonical Ltd. This software is licensed under the
28 * GNU Affero General Public License version 3 (see the file LICENSE). */
29
30 YUI.add('lp.app.picker.test', function (Y) {
31@@ -33,7 +33,7 @@
32 }
33
34 tests.suite.add(new Y.Test.Case({
35- name: 'picker_yesyno_validation',
36+ name: 'picker_yesno_validation',
37
38 setUp: function() {
39 this.vocabulary = [
40@@ -514,6 +514,39 @@
41
42 tests.suite.add(new Y.Test.Case({
43
44+ name: 'picker_custom_context_path',
45+
46+ create_picker: function(yio, extra_config) {
47+ var config = {yio: yio};
48+ if (extra_config !== undefined) {
49+ config = Y.merge(extra_config, config);
50+ }
51+ return Y.lp.app.picker.addPickerPatcher(
52+ "Foo",
53+ "foo/bar",
54+ "test_link",
55+ "picker_id",
56+ config);
57+ },
58+
59+ test_custom_context_path_honoured: function() {
60+ var mock_io = new Y.lp.testing.mockio.MockIo();
61+ var extra_config = {
62+ "getContextPath": function() { return "/gentoo"; }
63+ };
64+ var picker = this.create_picker(mock_io, extra_config);
65+ picker.fire('search', 'package');
66+ Assert.areEqual(1, mock_io.requests.length);
67+ Assert.areEqual(
68+ '/gentoo/@@+huge-vocabulary?name=Foo&search_text=package&' +
69+ 'batch=6&start=0', mock_io.requests[0].url);
70+ cleanup_widget(picker);
71+ }
72+
73+ }));
74+
75+ tests.suite.add(new Y.Test.Case({
76+
77 name: 'picker_error_handling',
78
79 setUp: function() {
80
81=== modified file 'lib/lp/app/widgets/popup.py'
82--- lib/lp/app/widgets/popup.py 2013-04-10 08:01:20 +0000
83+++ lib/lp/app/widgets/popup.py 2016-07-27 17:22:45 +0000
84@@ -1,4 +1,4 @@
85-# Copyright 2009 Canonical Ltd. This software is licensed under the
86+# Copyright 2009-2016 Canonical Ltd. This software is licensed under the
87 # GNU Affero General Public License version 3 (see the file LICENSE).
88
89 """Single selection widget using a popup to select one item from many."""
90@@ -275,3 +275,16 @@
91 "looking for? "
92 '<a href="%s/+affects-new-product">Register it</a>.</strong>'
93 % canonical_url(self.context.context))
94+
95+
96+class DistributionSourcePackagePickerWidget(VocabularyPickerWidget):
97+ """Custom popup widget for choosing distribution/package combinations."""
98+
99+ __call__ = ViewPageTemplateFile(
100+ 'templates/distributionsourcepackage-picker.pt')
101+
102+ @property
103+ def distribution_id(self):
104+ return self._prefix + 'distribution'
105+
106+ distribution_name = ''
107
108=== added file 'lib/lp/app/widgets/templates/distributionsourcepackage-picker.pt'
109--- lib/lp/app/widgets/templates/distributionsourcepackage-picker.pt 1970-01-01 00:00:00 +0000
110+++ lib/lp/app/widgets/templates/distributionsourcepackage-picker.pt 2016-07-27 17:22:45 +0000
111@@ -0,0 +1,29 @@
112+<tal:root
113+ xmlns:tal="http://xml.zope.org/namespaces/tal"
114+ xmlns:metal="http://xml.zope.org/namespaces/metal"
115+ omit-tag="">
116+
117+<metal:form-picker use-macro="context/@@form-picker-macros/form-picker">
118+ <script metal:fill-slot="add-picker" tal:content="structure string:
119+ LPJS.use('node', 'lp.app.picker', function(Y) {
120+ var config = ${view/json_config};
121+ var distribution_name = '${view/distribution_name}';
122+ var distribution_id = '${view/distribution_id}';
123+ if (distribution_name !== '') {
124+ config.getContextPath = function() {
125+ return '/' + distribution_name;
126+ }
127+ } else if (distribution_id !== '') {
128+ config.getContextPath = function() {
129+ return '/' + Y.DOM.byId(distribution_id).value;
130+ };
131+ }
132+ var show_widget_id = '${view/show_widget_id}';
133+ Y.on('domready', function(e) {
134+ Y.lp.app.picker.addPicker(config, show_widget_id);
135+ });
136+ });
137+ "/>
138+</metal:form-picker>
139+
140+</tal:root>
141
142=== modified file 'lib/lp/app/widgets/templates/form-picker-macros.pt'
143--- lib/lp/app/widgets/templates/form-picker-macros.pt 2012-02-01 23:47:54 +0000
144+++ lib/lp/app/widgets/templates/form-picker-macros.pt 2016-07-27 17:22:45 +0000
145@@ -29,7 +29,7 @@
146 </tal:search_results>
147
148 <tal:chooseLink replace="structure view/chooseLink" />
149- <script tal:content="structure string:
150+ <script metal:define-slot="add-picker" tal:content="structure string:
151 LPJS.use('node', 'lp.app.picker', function(Y) {
152 var config = ${view/json_config};
153 var show_widget_id = '${view/show_widget_id}';
154
155=== modified file 'lib/lp/app/widgets/tests/test_launchpadtarget.py'
156--- lib/lp/app/widgets/tests/test_launchpadtarget.py 2016-07-11 22:35:59 +0000
157+++ lib/lp/app/widgets/tests/test_launchpadtarget.py 2016-07-27 17:22:45 +0000
158@@ -82,7 +82,7 @@
159 # The render template is setup.
160 self.assertTrue(
161 self.widget.template.filename.endswith('launchpad-target.pt'),
162- 'Template was not setup.')
163+ 'Template was not set up.')
164
165 def test_default_option(self):
166 # This package field is the default option.
167@@ -198,20 +198,10 @@
168 self.widget.request = LaunchpadTestRequest(form=self.form)
169 self.assertEqual(self.package, self.widget.getInputValue())
170
171- def test_getInputValue_package_spn_dsp_picker_feature_flag(self):
172- # The field value is the package when the package radio button
173- # is selected and the package sub field has a official dsp.
174- self.widget.request = LaunchpadTestRequest(form=self.form)
175- with FeatureFixture({u"disclosure.dsp_picker.enabled": u"on"}):
176- self.widget.setUpSubWidgets()
177- self.assertEqual(self.package, self.widget.getInputValue())
178-
179 def test_getInputValue_package_dsp_dsp_picker_feature_flag(self):
180 # The field value is the package when the package radio button
181 # is selected and the package sub field has valid input.
182- form = self.form
183- form['field.target.package'] = 'fnord/snarf'
184- self.widget.request = LaunchpadTestRequest(form=form)
185+ self.widget.request = LaunchpadTestRequest(form=self.form)
186 with FeatureFixture({u"disclosure.dsp_picker.enabled": u"on"}):
187 self.widget.setUpSubWidgets()
188 self.assertEqual(self.package, self.widget.getInputValue())
189
190=== modified file 'lib/lp/app/widgets/tests/test_popup.py'
191--- lib/lp/app/widgets/tests/test_popup.py 2012-08-28 04:35:13 +0000
192+++ lib/lp/app/widgets/tests/test_popup.py 2016-07-27 17:22:45 +0000
193@@ -1,4 +1,4 @@
194-# Copyright 2010-2011 Canonical Ltd. This software is licensed under the
195+# Copyright 2010-2016 Canonical Ltd. This software is licensed under the
196 # GNU Affero General Public License version 3 (see the file LICENSE).
197
198 __metaclass__ = type
199@@ -10,6 +10,7 @@
200 from zope.schema.vocabulary import getVocabularyRegistry
201
202 from lp.app.widgets.popup import (
203+ DistributionSourcePackagePickerWidget,
204 PersonPickerWidget,
205 VocabularyPickerWidget,
206 )
207@@ -23,12 +24,11 @@
208 def __init__(self, name, bases=(), attrs=None, __doc__=None,
209 __module__=None):
210 attrs = {
211- "test_invalid_chars+":
212- Choice(vocabulary='ValidTeamOwner'),
213- "test_valid.item":
214- Choice(vocabulary='ValidTeamOwner'),
215- "test_filtered.item":
216- Choice(vocabulary='DistributionOrProduct')}
217+ "test_invalid_chars+": Choice(vocabulary='ValidTeamOwner'),
218+ "test_valid.item": Choice(vocabulary='ValidTeamOwner'),
219+ "test_filtered.item": Choice(vocabulary='DistributionOrProduct'),
220+ "test_target": Choice(vocabulary='DistributionSourcePackage'),
221+ }
222 super(TestMetaClass, self).__init__(
223 name, bases=bases, attrs=attrs, __doc__=__doc__,
224 __module__=__module__)
225@@ -197,3 +197,29 @@
226 person_picker_widget.setRenderedValue(team)
227 self.assertEqual('team',
228 person_picker_widget.config['selected_value_metadata'])
229+
230+ def test_distribution_source_package_widget_distribution_id(self):
231+ # The distribution source package picker refers to the correct field
232+ # in distribution_id.
233+ field = ITest['test_target']
234+ bound_field = field.bind(self.context)
235+ vocabulary = self.vocabulary_registry.get(
236+ self.context, 'DistributionSourcePackage')
237+ dsp_picker_widget = DistributionSourcePackagePickerWidget(
238+ bound_field, vocabulary, self.request)
239+ dsp_picker_widget.setPrefix('field.target')
240+ self.assertEqual(
241+ 'field.target.distribution', dsp_picker_widget.distribution_id)
242+
243+ def test_distribution_source_package_widget_gets_distribution(self):
244+ # The distribution source package picker gets the value of the
245+ # distribution field.
246+ field = ITest['test_target']
247+ bound_field = field.bind(self.context)
248+ vocabulary = self.vocabulary_registry.get(
249+ self.context, 'DistributionSourcePackage')
250+ dsp_picker_widget = DistributionSourcePackagePickerWidget(
251+ bound_field, vocabulary, self.request)
252+ dsp_picker_widget.setPrefix('field.target')
253+ markup = dsp_picker_widget()
254+ self.assertIn("distribution_id = 'field.target.distribution'", markup)
255
256=== modified file 'lib/lp/bugs/browser/tests/test_bugalsoaffects.py'
257--- lib/lp/bugs/browser/tests/test_bugalsoaffects.py 2016-07-11 22:35:59 +0000
258+++ lib/lp/bugs/browser/tests/test_bugalsoaffects.py 2016-07-27 17:22:45 +0000
259@@ -54,24 +54,7 @@
260 browser = self.openBugPage(bug)
261 browser.getLink(url='+distrotask').click()
262 browser.getControl('Distribution').value = [self.distribution.name]
263- browser.getControl('Source Package Name').value = (
264- dsp.sourcepackagename.name)
265- browser.getControl('Continue').click()
266- self.assertEqual([], get_feedback_messages(browser.contents))
267-
268- def test_bug_alsoaffects_dsp_exists_dsp_picker_feature_flag(self):
269- # If the distribution source package is official, there is no error.
270- bug = self.factory.makeBug()
271- distroseries = self.factory.makeDistroSeries(
272- distribution=self.distribution)
273- dsp = self.factory.makeDSPCache(
274- distroseries=distroseries, sourcepackagename='snarf')
275- with FeatureFixture({u"disclosure.dsp_picker.enabled": u"on"}):
276- browser = self.openBugPage(bug)
277- browser.getLink(url='+distrotask').click()
278- browser.getControl('Distribution').value = [self.distribution.name]
279- browser.getControl('Source Package Name').value = (
280- '%s/%s' % (self.distribution.name, dsp.name))
281+ browser.getControl('Source Package Name').value = dsp.name
282 browser.getControl('Continue').click()
283 self.assertEqual([], get_feedback_messages(browser.contents))
284
285
286=== modified file 'lib/lp/bugs/browser/widgets/bugtask.py'
287--- lib/lp/bugs/browser/widgets/bugtask.py 2016-06-10 22:06:13 +0000
288+++ lib/lp/bugs/browser/widgets/bugtask.py 2016-07-27 17:22:45 +0000
289@@ -53,8 +53,8 @@
290 from lp.app.widgets.helpers import get_widget_template
291 from lp.app.widgets.launchpadtarget import LaunchpadTargetWidget
292 from lp.app.widgets.popup import (
293+ DistributionSourcePackagePickerWidget,
294 PersonPickerWidget,
295- VocabularyPickerWidget,
296 )
297 from lp.app.widgets.textwidgets import (
298 StrippedTextWidget,
299@@ -478,12 +478,17 @@
300 return vocabulary
301
302
303-class BugTaskSourcePackageNameWidget(VocabularyPickerWidget):
304+class BugTaskSourcePackageNameWidget(DistributionSourcePackagePickerWidget):
305 """A widget for associating a bugtask with a SourcePackageName.
306
307 It accepts both binary and source package names.
308 """
309
310+ # Pages that use this widget don't display the distribution, but this
311+ # can only be used by bugtasks on the distribution in question so the
312+ # vocabulary will be able to work it out for itself.
313+ distribution_id = ''
314+
315 def __init__(self, field, vocabulary, request):
316 super(BugTaskSourcePackageNameWidget, self).__init__(
317 field, vocabulary, request)
318@@ -542,6 +547,8 @@
319 except that it gets the distribution from the request.
320 """
321
322+ distribution_id = 'field.distribution'
323+
324 def getDistribution(self):
325 """See `BugTaskSourcePackageNameWidget`"""
326 distribution_name = self.request.form.get('field.distribution')
327@@ -559,6 +566,8 @@
328 class UbuntuSourcePackageNameWidget(BugTaskSourcePackageNameWidget):
329 """A widget to select Ubuntu packages."""
330
331+ distribution_name = 'ubuntu'
332+
333 def getDistribution(self):
334 """See `BugTaskSourcePackageNameWidget`"""
335 return getUtility(ILaunchpadCelebrities).ubuntu
336
337=== modified file 'lib/lp/registry/tests/test_distributionsourcepackage_vocabulary.py'
338--- lib/lp/registry/tests/test_distributionsourcepackage_vocabulary.py 2016-07-11 22:35:59 +0000
339+++ lib/lp/registry/tests/test_distributionsourcepackage_vocabulary.py 2016-07-27 17:22:45 +0000
340@@ -69,52 +69,24 @@
341 vocabulary.setDistribution(new_distro)
342 self.assertEqual(new_distro, vocabulary.distribution)
343
344- def test_parseToken_distro_and_package(self):
345- # parseToken returns a tuple of distribution and package name when
346- # the text contains both.
347- new_distro = self.factory.makeDistribution(name='fnord')
348- vocabulary = DistributionSourcePackageVocabulary(None)
349- distribution, package_name = vocabulary.parseToken('fnord/pting')
350- self.assertEqual(new_distro, distribution)
351- self.assertEqual('pting', package_name)
352-
353- def test_parseToken_default_distro_and_package(self):
354- # parseToken returns a tuple of the default distribution and package
355- # name when the text is just a package name.
356- default_distro = self.factory.makeDistribution(name='fnord')
357- vocabulary = DistributionSourcePackageVocabulary(default_distro)
358- distribution, package_name = vocabulary.parseToken('pting')
359- self.assertEqual(default_distro, distribution)
360- self.assertEqual('pting', package_name)
361-
362- def test_parseToken_bad_distro_and_package(self):
363- # parseToken returns a tuple of the default distribution and package
364- # name when the distro in the text cannot be matched to a real
365- # distro.
366- default_distro = self.factory.makeDistribution(name='fnord')
367- vocabulary = DistributionSourcePackageVocabulary(default_distro)
368- distribution, package_name = vocabulary.parseToken('misspelled/pting')
369- self.assertEqual(default_distro, distribution)
370- self.assertEqual('pting', package_name)
371-
372- def test_contains_true_without_init(self):
373+ def test_contains_true_with_distribution(self):
374 # The vocabulary contains official DSPs.
375 dsp = self.factory.makeDistributionSourcePackage(with_db=True)
376- vocabulary = DistributionSourcePackageVocabulary(None)
377+ vocabulary = DistributionSourcePackageVocabulary(dsp.distribution)
378 self.assertIn(dsp, vocabulary)
379
380- def test_contains_true_with_init(self):
381+ def test_contains_true_with_dsp(self):
382 # The vocabulary contains the DSP passed to init when it is not
383 # official.
384 dsp = self.factory.makeDistributionSourcePackage(with_db=False)
385 vocabulary = DistributionSourcePackageVocabulary(dsp)
386 self.assertIn(dsp, vocabulary)
387
388- def test_contains_false_without_init(self):
389+ def test_contains_false_with_distribution(self):
390 # The vocabulary does not contain DSPs that are not official that
391 # were not passed to init.
392 dsp = self.factory.makeDistributionSourcePackage(with_db=False)
393- vocabulary = DistributionSourcePackageVocabulary(None)
394+ vocabulary = DistributionSourcePackageVocabulary(dsp.distribution)
395 self.assertNotIn(dsp, vocabulary)
396
397 def test_toTerm_raises_error(self):
398@@ -123,11 +95,12 @@
399 dsp = self.factory.makeDistributionSourcePackage(
400 sourcepackagename='foo')
401 vocabulary = DistributionSourcePackageVocabulary(dsp.distribution)
402- self.assertRaises(LookupError, vocabulary.toTerm, dsp.name)
403+ self.assertRaises(LookupError, vocabulary.toTerm, dsp)
404
405 def test_toTerm_none_raises_error(self):
406 # An error is raised for an SPN that does not exist.
407- vocabulary = DistributionSourcePackageVocabulary(None)
408+ vocabulary = DistributionSourcePackageVocabulary(
409+ self.factory.makeDistribution())
410 self.assertRaises(LookupError, vocabulary.toTerm, 'nonexistent')
411
412 def test_toTerm_spn_and_default_distribution(self):
413@@ -137,21 +110,8 @@
414 spph.sourcepackagerelease.sourcepackagename)
415 vocabulary = DistributionSourcePackageVocabulary(dsp.distribution)
416 term = vocabulary.toTerm(dsp.sourcepackagename)
417- expected_token = '%s/%s' % (dsp.distribution.name, dsp.name)
418- self.assertEqual(expected_token, term.token)
419- self.assertEqual(expected_token, term.title)
420- self.assertEqual(dsp, term.value)
421-
422- def test_toTerm_spn_and_distribution(self):
423- # The distribution is used with the spn if it is passed.
424- spph = self.factory.makeSourcePackagePublishingHistory()
425- dsp = spph.distroseries.distribution.getSourcePackage(
426- spph.sourcepackagerelease.sourcepackagename)
427- vocabulary = DistributionSourcePackageVocabulary(None)
428- term = vocabulary.toTerm(dsp.sourcepackagename, dsp.distribution)
429- expected_token = '%s/%s' % (dsp.distribution.name, dsp.name)
430- self.assertEqual(expected_token, term.token)
431- self.assertEqual(expected_token, term.title)
432+ self.assertEqual(dsp.name, term.token)
433+ self.assertEqual(dsp.name, term.title)
434 self.assertEqual(dsp, term.value)
435
436 def test_toTerm_dsp(self):
437@@ -161,9 +121,8 @@
438 spph.sourcepackagerelease.sourcepackagename)
439 vocabulary = DistributionSourcePackageVocabulary(dsp)
440 term = vocabulary.toTerm(dsp)
441- expected_token = '%s/%s' % (dsp.distribution.name, dsp.name)
442- self.assertEqual(expected_token, term.token)
443- self.assertEqual(expected_token, term.title)
444+ self.assertEqual(dsp.name, term.token)
445+ self.assertEqual(dsp.name, term.title)
446 self.assertEqual(dsp, term.value)
447
448 def test_toTerm_dsp_and_binary_names(self):
449@@ -174,9 +133,8 @@
450 spph.sourcepackagerelease.sourcepackagename)
451 vocabulary = DistributionSourcePackageVocabulary(dsp)
452 term = vocabulary.toTerm((dsp, 'one two'))
453- expected_token = '%s/%s' % (dsp.distribution.name, dsp.name)
454- self.assertEqual(expected_token, term.token)
455- self.assertEqual(expected_token, term.title)
456+ self.assertEqual(dsp.name, term.token)
457+ self.assertEqual(dsp.name, term.title)
458 self.assertEqual(dsp, term.value)
459 self.assertEqual(['one', 'two'], term.value.binary_names)
460
461@@ -185,8 +143,7 @@
462 dsp = self.factory.makeDistributionSourcePackage(
463 sourcepackagename='foo')
464 vocabulary = DistributionSourcePackageVocabulary(dsp.distribution)
465- token = '%s/%s' % (dsp.distribution.name, dsp.name)
466- self.assertRaises(LookupError, vocabulary.getTermByToken, token)
467+ self.assertRaises(LookupError, vocabulary.getTermByToken, dsp.name)
468
469 def test_getTermByToken_token(self):
470 # The term is returned if it matches an official DSP.
471@@ -194,20 +151,16 @@
472 dsp = spph.distroseries.distribution.getSourcePackage(
473 spph.sourcepackagerelease.sourcepackagename)
474 vocabulary = DistributionSourcePackageVocabulary(dsp.distribution)
475- token = '%s/%s' % (dsp.distribution.name, dsp.name)
476- term = vocabulary.getTermByToken(token)
477+ term = vocabulary.getTermByToken(dsp.name)
478 self.assertEqual(dsp, term.value)
479
480 def test_searchForTerms_without_distribution(self):
481- # An empty result set is returned if the vocabulary has no
482- # distribution and the search does not provide distribution
483- # information.
484+ # searchForTerms asserts that the vocabulary has a distribution.
485 spph = self.factory.makeSourcePackagePublishingHistory()
486 dsp = spph.distroseries.distribution.getSourcePackage(
487 spph.sourcepackagerelease.sourcepackagename)
488 vocabulary = DistributionSourcePackageVocabulary(dsp.name)
489- results = vocabulary.searchForTerms(dsp.name)
490- self.assertEqual(0, results.count())
491+ self.assertRaises(AssertionError, vocabulary.searchForTerms, dsp.name)
492
493 def test_searchForTerms_None(self):
494 # Searching for nothing gets you that.
495@@ -222,11 +175,11 @@
496 distroseries = self.factory.makeDistroSeries(distribution=distro)
497 self.factory.makeDSPCache(
498 distroseries=distroseries, sourcepackagename='snarf')
499- vocabulary = DistributionSourcePackageVocabulary(None)
500- results = vocabulary.searchForTerms(query='fnord/snarf')
501+ vocabulary = DistributionSourcePackageVocabulary(distro)
502+ results = vocabulary.searchForTerms(query='snarf')
503 terms = list(results)
504 self.assertEqual(1, len(terms))
505- self.assertEqual('fnord/snarf', terms[0].token)
506+ self.assertEqual('snarf', terms[0].token)
507
508 def test_searchForTerms_similar_official_source_name(self):
509 # Partial source name matches are found.
510@@ -234,11 +187,11 @@
511 distroseries = self.factory.makeDistroSeries(distribution=distro)
512 self.factory.makeDSPCache(
513 distroseries=distroseries, sourcepackagename='pting-snarf-ack')
514- vocabulary = DistributionSourcePackageVocabulary(None)
515- results = vocabulary.searchForTerms(query='fnord/snarf')
516+ vocabulary = DistributionSourcePackageVocabulary(distro)
517+ results = vocabulary.searchForTerms(query='snarf')
518 terms = list(results)
519 self.assertEqual(1, len(terms))
520- self.assertEqual('fnord/pting-snarf-ack', terms[0].token)
521+ self.assertEqual('pting-snarf-ack', terms[0].token)
522
523 def test_searchForTerms_exact_binary_name(self):
524 # Exact binary name matches are found.
525@@ -247,11 +200,11 @@
526 self.factory.makeDSPCache(
527 distroseries=distroseries, sourcepackagename='snarf',
528 binary_names='pting-dev pting ack')
529- vocabulary = DistributionSourcePackageVocabulary(None)
530- results = vocabulary.searchForTerms(query='fnord/pting')
531+ vocabulary = DistributionSourcePackageVocabulary(distro)
532+ results = vocabulary.searchForTerms(query='pting')
533 terms = list(results)
534 self.assertEqual(1, len(terms))
535- self.assertEqual('fnord/snarf', terms[0].token)
536+ self.assertEqual('snarf', terms[0].token)
537
538 def test_searchForTerms_similar_binary_name(self):
539 # Partial binary name matches are found.
540@@ -260,11 +213,11 @@
541 self.factory.makeDSPCache(
542 distroseries=distroseries, sourcepackagename='snarf',
543 binary_names='thrpp pting-dev ack')
544- vocabulary = DistributionSourcePackageVocabulary(None)
545- results = vocabulary.searchForTerms(query='fnord/pting')
546+ vocabulary = DistributionSourcePackageVocabulary(distro)
547+ results = vocabulary.searchForTerms(query='pting')
548 terms = list(results)
549 self.assertEqual(1, len(terms))
550- self.assertEqual('fnord/snarf', terms[0].token)
551+ self.assertEqual('snarf', terms[0].token)
552
553 def test_searchForTerms_exact_unofficial_source_name(self):
554 # Unofficial source packages are not found by search.
555@@ -273,8 +226,8 @@
556 self.factory.makeDSPCache(
557 distroseries=distroseries, sourcepackagename='snarf',
558 official=False)
559- vocabulary = DistributionSourcePackageVocabulary(None)
560- results = vocabulary.searchForTerms(query='fnord/snarf')
561+ vocabulary = DistributionSourcePackageVocabulary(distro)
562+ results = vocabulary.searchForTerms(query='snarf')
563 terms = list(results)
564 self.assertEqual(0, len(terms))
565
566@@ -285,8 +238,8 @@
567 self.factory.makeDSPCache(
568 distroseries=distroseries, sourcepackagename='snarf',
569 official=False, binary_names='thrpp pting ack')
570- vocabulary = DistributionSourcePackageVocabulary(None)
571- results = vocabulary.searchForTerms(query='fnord/pting')
572+ vocabulary = DistributionSourcePackageVocabulary(distro)
573+ results = vocabulary.searchForTerms(query='pting')
574 terms = list(results)
575 self.assertEqual(0, len(terms))
576
577@@ -304,14 +257,14 @@
578 self.factory.makeDSPCache(
579 distroseries=distroseries, sourcepackagename='pting-client',
580 binary_names='snarf-common')
581- vocabulary = DistributionSourcePackageVocabulary(None)
582- results = vocabulary.searchForTerms(query='fnord/snarf')
583+ vocabulary = DistributionSourcePackageVocabulary(distro)
584+ results = vocabulary.searchForTerms(query='snarf')
585 terms = list(results)
586 self.assertEqual(4, len(terms))
587- self.assertEqual('fnord/snarf', terms[0].token)
588- self.assertEqual('fnord/pting-devel', terms[1].token)
589- self.assertEqual('fnord/snarf-server', terms[2].token)
590- self.assertEqual('fnord/pting-client', terms[3].token)
591+ self.assertEqual('snarf', terms[0].token)
592+ self.assertEqual('pting-devel', terms[1].token)
593+ self.assertEqual('snarf-server', terms[2].token)
594+ self.assertEqual('pting-client', terms[3].token)
595
596 def test_searchForTerms_partner_archive(self):
597 # Packages in partner archives are searched.
598@@ -321,11 +274,11 @@
599 distroseries=distroseries, sourcepackagename='snarf',
600 archive=self.factory.makeArchive(
601 distribution=distro, purpose=ArchivePurpose.PARTNER))
602- vocabulary = DistributionSourcePackageVocabulary(None)
603- results = vocabulary.searchForTerms(query='fnord/snarf')
604+ vocabulary = DistributionSourcePackageVocabulary(distro)
605+ results = vocabulary.searchForTerms(query='snarf')
606 terms = list(results)
607 self.assertEqual(1, len(terms))
608- self.assertEqual('fnord/snarf', terms[0].token)
609+ self.assertEqual('snarf', terms[0].token)
610
611 def test_searchForTerms_ppa_archive(self):
612 # Packages in PPAs are ignored.
613@@ -336,6 +289,6 @@
614 official=False,
615 archive=self.factory.makeArchive(
616 distribution=distro, purpose=ArchivePurpose.PPA))
617- vocabulary = DistributionSourcePackageVocabulary(None)
618- results = vocabulary.searchForTerms(query='fnord/snarf')
619+ vocabulary = DistributionSourcePackageVocabulary(distro)
620+ results = vocabulary.searchForTerms(query='snarf')
621 self.assertEqual(0, results.count())
622
623=== modified file 'lib/lp/registry/vocabularies.py'
624--- lib/lp/registry/vocabularies.py 2016-07-11 21:46:03 +0000
625+++ lib/lp/registry/vocabularies.py 2016-07-27 17:22:45 +0000
626@@ -111,10 +111,7 @@
627 PersonVisibility,
628 )
629 from lp.registry.interfaces.accesspolicy import IAccessPolicySource
630-from lp.registry.interfaces.distribution import (
631- IDistribution,
632- IDistributionSet,
633- )
634+from lp.registry.interfaces.distribution import IDistribution
635 from lp.registry.interfaces.distributionsourcepackage import (
636 IDistributionSourcePackage,
637 )
638@@ -2028,7 +2025,7 @@
639 class DistributionSourcePackageVocabulary(FilteredVocabularyBase):
640
641 displayname = 'Select a package'
642- step_title = 'Search by name or distro/name'
643+ step_title = 'Search by name'
644
645 def __init__(self, context):
646 self.context = context
647@@ -2068,19 +2065,15 @@
648 """Set the distribution after the vocabulary was instantiated."""
649 self.distribution = distribution
650
651- def parseToken(self, text):
652- """Return the distribution and package name from the parsed token."""
653- # Match the toTerm() format, but also use it to select a distribution.
654- distribution = None
655- if '/' in text:
656- distro_name, text = text.split('/', 1)
657- distribution = getUtility(IDistributionSet).getByName(distro_name)
658- if distribution is None:
659- distribution = self.distribution
660- return distribution, text
661+ def _assertHasDistribution(self):
662+ if self.distribution is None:
663+ raise AssertionError(
664+ "DistributionSourcePackageVocabulary cannot be used without "
665+ "setting a distribution.")
666
667- def toTerm(self, spn_or_dsp, distribution=None):
668+ def toTerm(self, spn_or_dsp):
669 """See `IVocabulary`."""
670+ self._assertHasDistribution()
671 dsp = None
672 binary_names = None
673 if isinstance(spn_or_dsp, tuple):
674@@ -2088,21 +2081,27 @@
675 spn_or_dsp, binary_names = spn_or_dsp
676 if binary_names is not None:
677 binary_names = binary_names.split()
678+ # XXX cjwatson 2016-07-27: Eventually this should just take a DSP
679+ # and drop the complication of also accepting SPNs; but, for now,
680+ # accepting an SPN reduces the amount of feature-flag checks
681+ # required by users of this vocabulary.
682 if IDistributionSourcePackage.providedBy(spn_or_dsp):
683 dsp = spn_or_dsp
684- distribution = spn_or_dsp.distribution
685- else:
686- distribution = distribution or self.distribution
687- if distribution is not None and spn_or_dsp is not None:
688- dsp = distribution.getSourcePackage(spn_or_dsp)
689+ elif spn_or_dsp is not None:
690+ dsp = self.distribution.getSourcePackage(spn_or_dsp)
691 if dsp is not None and (dsp == self.dsp or dsp.is_official):
692 if binary_names:
693 # Search already did the hard work of looking up binary names.
694 cache = get_property_cache(dsp)
695 cache.binary_names = binary_names
696- token = '%s/%s' % (dsp.distribution.name, dsp.name)
697- return SimpleTerm(dsp, token, token)
698- raise LookupError(distribution, spn_or_dsp)
699+ # XXX cjwatson 2016-07-22: It's a bit odd for the token to
700+ # return just the source package name and not the distribution
701+ # name as well, but at the moment this is always fed into a
702+ # package name box so things work much better this way. If we
703+ # ever do a true combined distribution/package picker, then this
704+ # may need to be revisited.
705+ return SimpleTerm(dsp, dsp.name, dsp.name)
706+ raise LookupError(self.distribution, spn_or_dsp)
707
708 def getTerm(self, spn_or_dsp):
709 """See `IBaseVocabulary`."""
710@@ -2110,19 +2109,13 @@
711
712 def getTermByToken(self, token):
713 """See `IVocabularyTokenized`."""
714- distribution, package_name = self.parseToken(token)
715- return self.toTerm(package_name, distribution)
716+ return self.toTerm(token)
717
718 def searchForTerms(self, query=None, vocab_filter=None):
719 """See `IHugeVocabulary`."""
720+ self._assertHasDistribution()
721 if not query:
722 return EmptyResultSet()
723- distribution, query = self.parseToken(query)
724- if distribution is None:
725- # This could failover to ubuntu, but that is non-obvious. The
726- # Python widget must set the default distribution and the JS
727- # widget must encourage the <distro>/<package> search format.
728- return EmptyResultSet()
729
730 query = unicode(query)
731 query_re = re.escape(query)
732@@ -2143,9 +2136,10 @@
733 ),
734 Or(
735 DistributionSourcePackageCache.archiveID.is_in(
736- distribution.all_distro_archive_ids),
737+ self.distribution.all_distro_archive_ids),
738 DistributionSourcePackageCache.archive == None),
739- DistributionSourcePackageCache.distribution == distribution,
740+ DistributionSourcePackageCache.distribution ==
741+ self.distribution,
742 ),
743 tables=DistributionSourcePackageCache))
744 SearchableDSPC = Table("SearchableDSPC")
745@@ -2178,7 +2172,7 @@
746 (DistributionSourcePackageInDatabase,
747 searchable_dspc_binpkgnames),
748 DistributionSourcePackageInDatabase.distribution ==
749- distribution,
750+ self.distribution,
751 DistributionSourcePackageInDatabase.sourcepackagename_id ==
752 searchable_dspc_sourcepackagename)
753 results.order_by(Desc(rank), searchable_dspc_name)
754
755=== modified file 'lib/lp/services/webapp/configure.zcml'
756--- lib/lp/services/webapp/configure.zcml 2014-11-28 22:07:05 +0000
757+++ lib/lp/services/webapp/configure.zcml 2016-07-27 17:22:45 +0000
758@@ -1,4 +1,4 @@
759-<!-- Copyright 2009-2011 Canonical Ltd. This software is licensed under the
760+<!-- Copyright 2009-2016 Canonical Ltd. This software is licensed under the
761 GNU Affero General Public License version 3 (see the file LICENSE).
762 -->
763
764@@ -413,6 +413,17 @@
765 permission="zope.Public"
766 />
767
768+ <!-- Define the widget used by fields that use
769+ DistributionSourcePackageVocabulary. -->
770+ <view
771+ type="zope.publisher.interfaces.browser.IBrowserRequest"
772+ for="zope.schema.interfaces.IChoice
773+ lp.registry.vocabularies.DistributionSourcePackageVocabulary"
774+ provides="zope.formlib.interfaces.IInputWidget"
775+ factory="lp.app.widgets.popup.DistributionSourcePackagePickerWidget"
776+ permission="zope.Public"
777+ />
778+
779 <!-- A simple view used by the page tests. -->
780 <browser:page
781 for="lp.services.webapp.interfaces.ILaunchpadRoot"