Merge lp:~cjwatson/launchpad/fix-dsp-vocab-picker into lp:launchpad
- fix-dsp-vocab-picker
- Merge into devel
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 | ||||||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
William Grant | code | Approve | |
Review via email: mp+300782@code.launchpad.net |
Commit message
Customise the picker for DistributionSou
Description of the change
Customise the picker for DistributionSou
I've also changed the vocabulary itself to return just the package name as the term's token and title, rather than distribution/
William Grant (wgrant) : | # |
Preview Diff
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" |