Launchpad itself

Merge lp:~edwin-grubbs/launchpad/bug-433809-picker-search-slash into lp:launchpad/db-devel

  1. bug-433809-picker-search-slash
  2. Merge into db-devel
Proposed by Edwin Grubbs
Status: Merged
Approved by: Aaron Bentley
Approved revision: not available
Merged at revision: not available
Proposed branch: lp:~edwin-grubbs/launchpad/bug-433809-picker-search-slash
Merge into: lp:launchpad/db-devel
Diff against target: 150 lines (+100/-20)
2 files modified
lib/lp/registry/tests/test_productseries_vocabularies.py (+72/-0)
lib/lp/registry/vocabularies.py (+28/-20)
To merge this branch: bzr merge lp:~edwin-grubbs/launchpad/bug-433809-picker-search-slash
Reviewer Review Type Date Requested Status
Aaron Bentley (community) Approve
Canonical Launchpad Engineering code Pending
Review via email: mp+15674@code.launchpad.net

Commit message

Fixed ProductSeriesVocabulary searches containing a slash.

To post a comment you must log in.
Revision history for this message
Edwin Grubbs (edwin-grubbs) wrote :

Summary
-------

This bug fixes the problem where adding a slash to the search for
a project series always yields zero results.

Since I converted ProductSeriesVocabulary.search() to use storm
directly instead of the sqlobject compatibility layer, I updated
the getTermByToken() method to also use storm.

This will be landed on devel, but since devel still hasn't had
db-devel's changes for 3.1.11 merged in, I'm targeting this
merge proposal to db-devel so that the automatic diff will be correct.

Tests
-----

./bin/test -vv lp.registry.tests.test_productseries_vocabularies

Demo and Q/A
------------

* Open https://launchpad.dev/ubuntu/hoary/+source/alsa-utils/+edit-packaging
  * Click the "Choose..." link.
  * Searching for "fire/" should show two matching series.
  * Searching for "fire/t" should just show 'firefox trunk'.

Revision history for this message
Edwin Grubbs (edwin-grubbs) wrote :

Hi Aaron,

Here is an incremental diff which corrects how errors are handled since I had changed the sqlobject selectOne() call to storm's find().

{{{
=== modified file 'lib/lp/registry/tests/test_productseries_vocabularies.py'
--- lib/lp/registry/tests/test_productseries_vocabularies.py 2009-12-04 19:26:58 +0000
+++ lib/lp/registry/tests/test_productseries_vocabularies.py 2009-12-07 20:13:21 +0000
@@ -62,6 +62,11 @@
         self.assertEqual(token, term.token)
         self.assertEqual(self.series, term.value)

+ def test_getTermByToken_LookupError(self):
+ self.assertRaises(
+ LookupError,
+ self.vocabulary.getTermByToken, 'does/notexist')
+

 def test_suite():
     return TestLoader().loadTestsFromName(__name__)

=== modified file 'lib/lp/registry/vocabularies.py'
--- lib/lp/registry/vocabularies.py 2009-12-04 20:40:30 +0000
+++ lib/lp/registry/vocabularies.py 2009-12-07 20:11:24 +0000
@@ -1015,9 +1015,9 @@
             self._table,
             ProductSeries.product == Product.id,
             Product.name == productname,
- ProductSeries.name == productseriesname)
+ ProductSeries.name == productseriesname).one()
         if result is not None:
- return self.toTerm(result.one())
+ return self.toTerm(result)
         raise LookupError(token)

     def search(self, query):

}}}

Revision history for this message
Aaron Bentley (abentley) wrote :

> Here is an incremental diff which corrects how errors are handled since I had
> changed the sqlobject selectOne() call to storm's find().

Cool, thanks!

Aaron

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== added file 'lib/lp/registry/tests/test_productseries_vocabularies.py'
2--- lib/lp/registry/tests/test_productseries_vocabularies.py 1970-01-01 00:00:00 +0000
3+++ lib/lp/registry/tests/test_productseries_vocabularies.py 2009-12-07 20:16:12 +0000
4@@ -0,0 +1,72 @@
5+# Copyright 2009 Canonical Ltd. This software is licensed under the
6+# GNU Affero General Public License version 3 (see the file LICENSE).
7+
8+"""Test the milestone vocabularies."""
9+
10+__metaclass__ = type
11+
12+from unittest import TestLoader
13+from operator import attrgetter
14+
15+from lp.registry.vocabularies import ProductSeriesVocabulary
16+from lp.testing import TestCaseWithFactory
17+
18+from canonical.testing import DatabaseFunctionalLayer
19+
20+
21+class TestProductSeriesVocabulary(TestCaseWithFactory):
22+ """Test that the ProductSeriesVocabulary behaves as expected."""
23+ layer = DatabaseFunctionalLayer
24+
25+ def setUp(self):
26+ super(TestProductSeriesVocabulary, self).setUp()
27+ self.vocabulary = ProductSeriesVocabulary()
28+ self.product_prefix = 'asdf987-'
29+ self.series1_prefix = 'qwerty-'
30+ self.product = self.factory.makeProduct(
31+ self.product_prefix + 'product1')
32+ self.series = self.factory.makeProductSeries(
33+ product=self.product, name=self.series1_prefix + "series1")
34+
35+ def tearDown(self):
36+ super(TestProductSeriesVocabulary, self).tearDown()
37+
38+ def test_search(self):
39+ series2 = self.factory.makeProductSeries(product=self.product)
40+ # Search by product name.
41+ result = self.vocabulary.search(self.product.name)
42+ self.assertEqual(
43+ [self.series, series2].sort(key=attrgetter('id')),
44+ list(result).sort(key=attrgetter('id')))
45+ # Search by series name.
46+ result = self.vocabulary.search(self.series.name)
47+ self.assertEqual([self.series], list(result))
48+ # Search by series2 name.
49+ result = self.vocabulary.search(series2.name)
50+ self.assertEqual([series2], list(result))
51+ # Search by product & series name substrings.
52+ result = self.vocabulary.search(
53+ '%s/%s' % (self.product_prefix, self.series1_prefix))
54+ self.assertEqual([self.series], list(result))
55+
56+ def test_toTerm(self):
57+ term = self.vocabulary.toTerm(self.series)
58+ self.assertEqual(
59+ '%s/%s' % (self.product.name, self.series.name),
60+ term.token)
61+ self.assertEqual(self.series, term.value)
62+
63+ def test_getTermByToken(self):
64+ token = '%s/%s' % (self.product.name, self.series.name)
65+ term = self.vocabulary.getTermByToken(token)
66+ self.assertEqual(token, term.token)
67+ self.assertEqual(self.series, term.value)
68+
69+ def test_getTermByToken_LookupError(self):
70+ self.assertRaises(
71+ LookupError,
72+ self.vocabulary.getTermByToken, 'does/notexist')
73+
74+
75+def test_suite():
76+ return TestLoader().loadTestsFromName(__name__)
77
78=== modified file 'lib/lp/registry/vocabularies.py'
79--- lib/lp/registry/vocabularies.py 2009-11-30 20:22:23 +0000
80+++ lib/lp/registry/vocabularies.py 2009-12-07 20:16:12 +0000
81@@ -994,7 +994,7 @@
82
83 displayname = 'Select a Release Series'
84 _table = ProductSeries
85- _orderBy = [Product.q.name, ProductSeries.q.name]
86+ _order_by = [Product.name, ProductSeries.name]
87 _clauseTables = ['Product']
88
89 def toTerm(self, obj):
90@@ -1012,33 +1012,41 @@
91 except ValueError:
92 raise LookupError(token)
93
94- result = ProductSeries.selectOne('''
95- Product.id = ProductSeries.product AND
96- Product.name = %s AND
97- ProductSeries.name = %s
98- ''' % sqlvalues(productname, productseriesname),
99- clauseTables=['Product'])
100+ result = IStore(self._table).find(
101+ self._table,
102+ ProductSeries.product == Product.id,
103+ Product.name == productname,
104+ ProductSeries.name == productseriesname).one()
105 if result is not None:
106 return self.toTerm(result)
107 raise LookupError(token)
108
109 def search(self, query):
110- """Return terms where query is a substring of the name"""
111+ """Return terms where query is a substring of the name."""
112 if not query:
113 return self.emptySelectResults()
114
115- query = query.lower()
116- objs = self._table.select(
117- AND(
118- Product.q.id == ProductSeries.q.productID,
119- OR(
120- CONTAINSSTRING(Product.q.name, query),
121- CONTAINSSTRING(ProductSeries.q.name, query)
122- )
123- ),
124- orderBy=self._orderBy
125- )
126- return objs
127+ query = query.lower().strip('/')
128+ # If there is a slash splitting the product and productseries
129+ # names, they must both match. If there is no slash, we don't
130+ # know whether it is matching the product or the productseries
131+ # so we search both for the same string.
132+ if '/' in query:
133+ product_query, series_query = query.split('/', 1)
134+ substring_search = And(
135+ CONTAINSSTRING(Product.name, product_query),
136+ CONTAINSSTRING(ProductSeries.name, series_query))
137+ else:
138+ substring_search = Or(
139+ CONTAINSSTRING(Product.name, query),
140+ CONTAINSSTRING(ProductSeries.name, query))
141+
142+ result = IStore(self._table).find(
143+ self._table,
144+ Product.id == ProductSeries.productID,
145+ substring_search)
146+ result = result.order_by(self._order_by)
147+ return result
148
149
150 class FilteredDistroSeriesVocabulary(SQLObjectVocabularyBase):

Subscribers

People subscribed via source and target branches

to status/vote changes: