launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #04719
[Merge] lp:~wallyworld/launchpad/incorrect-picker-label-830935 into lp:launchpad
Ian Booth has proposed merging lp:~wallyworld/launchpad/incorrect-picker-label-830935 into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #830935 in Launchpad itself: "Incorrect label on picker filter"
https://bugs.launchpad.net/launchpad/+bug/830935
For more details, see:
https://code.launchpad.net/~wallyworld/launchpad/incorrect-picker-label-830935/+merge/72513
== Implementation ==
Fix the title attribute of the picker entries for projects and project groups so that they use the correct terminology. Introduce a pillar_category property to Product and ProjectGroup model classes to provide the relevant text.
The picker search results display-
Before:
Firefox (Product)
Mozilla (ProjectGroup)
After:
Firefox (Project)
Mozilla (Project Group)
The filter text has also been changed to match.
Also did some drive-by lint fixes and a little test cleanup.
== Demo/QA ==
http://people.canonical.com/~ianb/picker-filter-bug-fixed.png
== Tests ==
Added new tests to:
lp/app/widgets/tests/test_popup.py
lp/registry/tests/test_pillar_vocabularies.py
lp/registry/tests/test_product.py
lp/registry/tests/test_projectgroup.py
== Lint ==
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/app/widgets/tests/test_popup.py
lib/lp/registry/configure.zcml
lib/lp/registry/vocabularies.py
lib/lp/registry/model/product.py
lib/lp/registry/model/projectgroup.py
lib/lp/registry/tests/test_pillar_vocabularies.py
lib/lp/registry/tests/test_product.py
lib/lp/registry/tests/test_projectgroup.py
--
https://code.launchpad.net/~wallyworld/launchpad/incorrect-picker-label-830935/+merge/72513
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wallyworld/launchpad/incorrect-picker-label-830935 into lp:launchpad.
=== modified file 'lib/lp/app/widgets/tests/test_popup.py'
--- lib/lp/app/widgets/tests/test_popup.py 2011-08-22 13:01:14 +0000
+++ lib/lp/app/widgets/tests/test_popup.py 2011-08-23 00:36:29 +0000
@@ -103,9 +103,9 @@
'title': 'All',
'description': 'Display all search results'},
{'name': 'PROJECT',
- 'title': 'Product',
+ 'title': 'Project',
'description':
- 'Display search results associated with products'},
+ 'Display search results associated with projects'},
{'name': 'DISTRO',
'title': 'Distribution',
'description':
=== modified file 'lib/lp/registry/configure.zcml'
--- lib/lp/registry/configure.zcml 2011-08-18 20:35:41 +0000
+++ lib/lp/registry/configure.zcml 2011-08-23 00:36:29 +0000
@@ -367,6 +367,9 @@
<class
class="lp.registry.model.projectgroup.ProjectGroup">
<allow
+ attributes="
+ pillar_category"/>
+ <allow
interface="lp.registry.interfaces.projectgroup.IProjectGroupPublic"/>
<allow interface="lp.bugs.interfaces.bugsummary.IBugSummaryDimension"/>
<allow
@@ -1189,6 +1192,9 @@
<class
class="lp.registry.model.product.Product">
<allow
+ attributes="
+ pillar_category"/>
+ <allow
interface="lp.registry.interfaces.product.IProductPublic"/>
<allow interface="lp.bugs.interfaces.bugsummary.IBugSummaryDimension"/>
<allow
=== modified file 'lib/lp/registry/model/product.py'
--- lib/lp/registry/model/product.py 2011-06-21 01:34:08 +0000
+++ lib/lp/registry/model/product.py 2011-08-23 00:36:29 +0000
@@ -386,6 +386,10 @@
date_next_suggest_packaging = UtcDateTimeCol(default=None)
@property
+ def pillar_category(self):
+ return "Project"
+
+ @property
def official_codehosting(self):
# XXX Need to remove official_codehosting column from Product
# table.
=== modified file 'lib/lp/registry/model/projectgroup.py'
--- lib/lp/registry/model/projectgroup.py 2011-06-21 00:04:12 +0000
+++ lib/lp/registry/model/projectgroup.py 2011-08-23 00:36:29 +0000
@@ -76,7 +76,6 @@
HasBugHeatMixin,
OfficialBugTag,
)
-from lp.bugs.model.bugtask import BugTask
from lp.bugs.model.structuralsubscription import (
StructuralSubscriptionTargetMixin,
)
@@ -171,6 +170,10 @@
max_bug_heat = Int()
@property
+ def pillar_category(self):
+ return "Project Group"
+
+ @property
def products(self):
return Product.selectBy(
project=self, active=True, orderBy='displayname')
=== modified file 'lib/lp/registry/tests/test_pillar_vocabularies.py'
--- lib/lp/registry/tests/test_pillar_vocabularies.py 2011-08-18 00:46:19 +0000
+++ lib/lp/registry/tests/test_pillar_vocabularies.py 2011-08-23 00:36:29 +0000
@@ -35,14 +35,22 @@
self.vocabulary.supportedFilters()
)
- def test_toTerm(self):
+ def test_Product_toTerm(self):
# Product terms are composed of title, name, and the object.
term = self.vocabulary.toTerm(self.product)
- title = '%s (Product)' % self.product.title
+ title = '%s (Project)' % self.product.title
self.assertEqual(title, term.title)
self.assertEqual(self.product.name, term.token)
self.assertEqual(self.product, term.value)
+ def test_ProjectGroup_toTerm(self):
+ # ProductGroup terms are composed of title, name, and the object.
+ term = self.vocabulary.toTerm(self.project_group)
+ title = '%s (Project Group)' % self.project_group.title
+ self.assertEqual(title, term.title)
+ self.assertEqual(self.project_group.name, term.token)
+ self.assertEqual(self.project_group, term.value)
+
def test_getTermByToken(self):
# Tokens are case insentive because the product name is lowercase.
term = self.vocabulary.getTermByToken('ORCHID-SNARK')
=== modified file 'lib/lp/registry/tests/test_product.py'
--- lib/lp/registry/tests/test_product.py 2011-06-28 15:04:29 +0000
+++ lib/lp/registry/tests/test_product.py 2011-08-23 00:36:29 +0000
@@ -5,7 +5,6 @@
from cStringIO import StringIO
import datetime
-import unittest
from lazr.lifecycle.snapshot import Snapshot
import pytz
@@ -35,6 +34,7 @@
from lp.testing import (
login,
login_person,
+ TestCase,
TestCaseWithFactory,
WebServiceTestCase,
)
@@ -46,6 +46,11 @@
layer = DatabaseFunctionalLayer
+ def test_pillar_category(self):
+ # Products are really called Projects
+ product = self.factory.makeProduct()
+ self.assertEqual("Project", product.pillar_category)
+
def test_deactivation_failure(self):
# Ensure that a product cannot be deactivated if
# it is linked to source packages.
@@ -97,7 +102,7 @@
# milestones should be included.
product = self.factory.makeProduct(name='foo')
for i in range(25):
- milestone_list = self.factory.makeMilestone(
+ self.factory.makeMilestone(
product=product,
productseries=product.development_focus,
name=str(i))
@@ -181,7 +186,7 @@
[series.name for series in active_series])
-class TestProductFiles(unittest.TestCase):
+class TestProductFiles(TestCase):
"""Tests for downloadable product files."""
layer = LaunchpadFunctionalLayer
@@ -201,7 +206,7 @@
foo_file, 'text/plain', filename)
firefox_owner.getControl(name='field.signature').add_file(
foo_signature, 'text/plain', '%s.asc' % filename)
- firefox_owner.getControl('Description').value="Foo installer"
+ firefox_owner.getControl('Description').value = "Foo installer"
firefox_owner.getControl(name="field.contenttype").displayValue = \
["Installer file"]
firefox_owner.getControl("Upload").click()
@@ -235,20 +240,20 @@
self.assertEqual(a_element.contents[0].strip(), u'sig')
-class ProductAttributeCacheTestCase(unittest.TestCase):
+class ProductAttributeCacheTestCase(TestCase):
"""Cached attributes must be cleared at the end of a transaction."""
layer = DatabaseFunctionalLayer
def setUp(self):
+ super(ProductAttributeCacheTestCase, self).setUp()
self.product = Product.selectOneBy(name='tomcat')
def testLicensesCache(self):
"""License cache should be cleared automatically."""
self.assertEqual(self.product.licenses,
(License.ACADEMIC, License.AFFERO))
- product_license = ProductLicense(
- product=self.product, license=License.PYTHON)
+ ProductLicense(product=self.product, license=License.PYTHON)
# Cache doesn't see new value.
self.assertEqual(self.product.licenses,
(License.ACADEMIC, License.AFFERO))
@@ -258,8 +263,7 @@
# Cache is cleared and it sees database changes that occur
# before the cache is populated.
transaction.abort()
- product_license = ProductLicense(
- product=self.product, license=License.MIT)
+ ProductLicense(product=self.product, license=License.MIT)
self.assertEqual(self.product.licenses,
(License.ACADEMIC, License.AFFERO, License.MIT))
@@ -267,7 +271,7 @@
"""commercial_subscription cache should not traverse transactions."""
self.assertEqual(self.product.commercial_subscription, None)
now = datetime.datetime.now(pytz.UTC)
- subscription = CommercialSubscription(
+ CommercialSubscription(
product=self.product,
date_starts=now,
date_expires=now,
@@ -286,7 +290,7 @@
# Cache is cleared again.
transaction.abort()
- subscription = CommercialSubscription(
+ CommercialSubscription(
product=self.product,
date_starts=now,
date_expires=now,
@@ -378,7 +382,3 @@
ws_group = self.wsObject(group)
ws_product.translationgroup = ws_group
ws_product.lp_save()
-
-
-def test_suite():
- return unittest.TestLoader().loadTestsFromName(__name__)
=== modified file 'lib/lp/registry/tests/test_projectgroup.py'
--- lib/lp/registry/tests/test_projectgroup.py 2011-08-12 11:37:08 +0000
+++ lib/lp/registry/tests/test_projectgroup.py 2011-08-23 00:36:29 +0000
@@ -21,6 +21,17 @@
)
+class TestProjectGroup(TestCaseWithFactory):
+ """Tests project group object."""
+
+ layer = DatabaseFunctionalLayer
+
+ def test_pillar_category(self):
+ # The pillar category is correct.
+ pg = self.factory.makeProject()
+ self.assertEqual("Project Group", pg.pillar_category)
+
+
class ProjectGroupSearchTestCase(TestCaseWithFactory):
layer = LaunchpadFunctionalLayer
=== modified file 'lib/lp/registry/vocabularies.py'
--- lib/lp/registry/vocabularies.py 2011-08-22 12:13:22 +0000
+++ lib/lp/registry/vocabularies.py 2011-08-23 00:36:29 +0000
@@ -1994,8 +1994,8 @@
def __new__(cls):
return super(VocabularyFilter, cls).__new__(
- cls, 'PROJECT', 'Product',
- 'Display search results associated with products')
+ cls, 'PROJECT', 'Project',
+ 'Display search results associated with projects')
@property
def filter_terms(self):
@@ -2007,8 +2007,8 @@
def __new__(cls):
return super(VocabularyFilter, cls).__new__(
- cls, 'PROJECTGROUP', 'Project',
- 'Display search results associated with projects')
+ cls, 'PROJECTGROUP', 'Project Group',
+ 'Display search results associated with project groups')
@property
def filter_terms(self):
@@ -2050,9 +2050,10 @@
obj.__class__.__name__, obj.id)
obj = obj.pillar
- # It is a hack using the class name here, but it works
- # fine and avoids an ugly if statement.
- title = '%s (%s)' % (obj.title, obj.__class__.__name__)
+ # We use the "pillar_category" property if it exists, otherwise we
+ # just use the class name.
+ category = getattr(obj, "pillar_category", obj.__class__.__name__)
+ title = '%s (%s)' % (obj.title, category)
return SimpleTerm(obj, obj.name, title)
Follow ups