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
=== added file 'lib/lp/registry/tests/test_productseries_vocabularies.py'
--- lib/lp/registry/tests/test_productseries_vocabularies.py 1970-01-01 00:00:00 +0000
+++ lib/lp/registry/tests/test_productseries_vocabularies.py 2009-12-07 20:16:12 +0000
@@ -0,0 +1,72 @@
1# Copyright 2009 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).
3
4"""Test the milestone vocabularies."""
5
6__metaclass__ = type
7
8from unittest import TestLoader
9from operator import attrgetter
10
11from lp.registry.vocabularies import ProductSeriesVocabulary
12from lp.testing import TestCaseWithFactory
13
14from canonical.testing import DatabaseFunctionalLayer
15
16
17class TestProductSeriesVocabulary(TestCaseWithFactory):
18 """Test that the ProductSeriesVocabulary behaves as expected."""
19 layer = DatabaseFunctionalLayer
20
21 def setUp(self):
22 super(TestProductSeriesVocabulary, self).setUp()
23 self.vocabulary = ProductSeriesVocabulary()
24 self.product_prefix = 'asdf987-'
25 self.series1_prefix = 'qwerty-'
26 self.product = self.factory.makeProduct(
27 self.product_prefix + 'product1')
28 self.series = self.factory.makeProductSeries(
29 product=self.product, name=self.series1_prefix + "series1")
30
31 def tearDown(self):
32 super(TestProductSeriesVocabulary, self).tearDown()
33
34 def test_search(self):
35 series2 = self.factory.makeProductSeries(product=self.product)
36 # Search by product name.
37 result = self.vocabulary.search(self.product.name)
38 self.assertEqual(
39 [self.series, series2].sort(key=attrgetter('id')),
40 list(result).sort(key=attrgetter('id')))
41 # Search by series name.
42 result = self.vocabulary.search(self.series.name)
43 self.assertEqual([self.series], list(result))
44 # Search by series2 name.
45 result = self.vocabulary.search(series2.name)
46 self.assertEqual([series2], list(result))
47 # Search by product & series name substrings.
48 result = self.vocabulary.search(
49 '%s/%s' % (self.product_prefix, self.series1_prefix))
50 self.assertEqual([self.series], list(result))
51
52 def test_toTerm(self):
53 term = self.vocabulary.toTerm(self.series)
54 self.assertEqual(
55 '%s/%s' % (self.product.name, self.series.name),
56 term.token)
57 self.assertEqual(self.series, term.value)
58
59 def test_getTermByToken(self):
60 token = '%s/%s' % (self.product.name, self.series.name)
61 term = self.vocabulary.getTermByToken(token)
62 self.assertEqual(token, term.token)
63 self.assertEqual(self.series, term.value)
64
65 def test_getTermByToken_LookupError(self):
66 self.assertRaises(
67 LookupError,
68 self.vocabulary.getTermByToken, 'does/notexist')
69
70
71def test_suite():
72 return TestLoader().loadTestsFromName(__name__)
073
=== modified file 'lib/lp/registry/vocabularies.py'
--- lib/lp/registry/vocabularies.py 2009-11-30 20:22:23 +0000
+++ lib/lp/registry/vocabularies.py 2009-12-07 20:16:12 +0000
@@ -994,7 +994,7 @@
994994
995 displayname = 'Select a Release Series'995 displayname = 'Select a Release Series'
996 _table = ProductSeries996 _table = ProductSeries
997 _orderBy = [Product.q.name, ProductSeries.q.name]997 _order_by = [Product.name, ProductSeries.name]
998 _clauseTables = ['Product']998 _clauseTables = ['Product']
999999
1000 def toTerm(self, obj):1000 def toTerm(self, obj):
@@ -1012,33 +1012,41 @@
1012 except ValueError:1012 except ValueError:
1013 raise LookupError(token)1013 raise LookupError(token)
10141014
1015 result = ProductSeries.selectOne('''1015 result = IStore(self._table).find(
1016 Product.id = ProductSeries.product AND1016 self._table,
1017 Product.name = %s AND1017 ProductSeries.product == Product.id,
1018 ProductSeries.name = %s1018 Product.name == productname,
1019 ''' % sqlvalues(productname, productseriesname),1019 ProductSeries.name == productseriesname).one()
1020 clauseTables=['Product'])
1021 if result is not None:1020 if result is not None:
1022 return self.toTerm(result)1021 return self.toTerm(result)
1023 raise LookupError(token)1022 raise LookupError(token)
10241023
1025 def search(self, query):1024 def search(self, query):
1026 """Return terms where query is a substring of the name"""1025 """Return terms where query is a substring of the name."""
1027 if not query:1026 if not query:
1028 return self.emptySelectResults()1027 return self.emptySelectResults()
10291028
1030 query = query.lower()1029 query = query.lower().strip('/')
1031 objs = self._table.select(1030 # If there is a slash splitting the product and productseries
1032 AND(1031 # names, they must both match. If there is no slash, we don't
1033 Product.q.id == ProductSeries.q.productID,1032 # know whether it is matching the product or the productseries
1034 OR(1033 # so we search both for the same string.
1035 CONTAINSSTRING(Product.q.name, query),1034 if '/' in query:
1036 CONTAINSSTRING(ProductSeries.q.name, query)1035 product_query, series_query = query.split('/', 1)
1037 )1036 substring_search = And(
1038 ),1037 CONTAINSSTRING(Product.name, product_query),
1039 orderBy=self._orderBy1038 CONTAINSSTRING(ProductSeries.name, series_query))
1040 )1039 else:
1041 return objs1040 substring_search = Or(
1041 CONTAINSSTRING(Product.name, query),
1042 CONTAINSSTRING(ProductSeries.name, query))
1043
1044 result = IStore(self._table).find(
1045 self._table,
1046 Product.id == ProductSeries.productID,
1047 substring_search)
1048 result = result.order_by(self._order_by)
1049 return result
10421050
10431051
1044class FilteredDistroSeriesVocabulary(SQLObjectVocabularyBase):1052class FilteredDistroSeriesVocabulary(SQLObjectVocabularyBase):

Subscribers

People subscribed via source and target branches

to status/vote changes: