launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #04573
[Merge] lp:~jcsackett/launchpad/fix-picker-entry-adapters into lp:launchpad
j.c.sackett has proposed merging lp:~jcsackett/launchpad/fix-picker-entry-adapters into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #820005 in Launchpad itself: "target pickers could clearly state what they are"
https://bugs.launchpad.net/launchpad/+bug/820005
For more details, see:
https://code.launchpad.net/~jcsackett/launchpad/fix-picker-entry-adapters/+merge/71204
Summary
=======
As a series of enhancements to target (e.g. project, product, package) pickers, we need to update the adapters for targets to entry pickers. (See http://people.canonical.com/~curtis/target-picker/target-picker-0.html for a picture)
This branch is part of an incremental fix. It tackles the adapter for source packages; it updates the DistributionSourcePackage vocabulary to return DSPs instead of the sourcepackagename, and moves information provided by the vocabulary into the adapter, so that it can be properly displayed in the picker.
Preimplementation
=================
Spoke with Curtis Hovey.
Implementation
==============
lib/lp/app/browser/vocabulary.py
lib/lp/registry/vocabularies.py
lib/lp/app/browser/configure.zcml
---------------------------------
A new adapter for DistributionSourcePackages to PickerEntries has been created. The term summary from the vocabulary no longer includes the build history, since that information needs to be presented differently throughout LP. The build history is now added to picker.description in the new picker adapter.
The new adapter has been added to the zcml. The token returned by the vocabulary has also been updated to conform to our desired choice results in the LP UI.
lib/lp/registry/tests/test_dsp_vocabularies.py
----------------------------------------------
A number of tests were updated to reflect that the vocabulary now returns the actual distribution source package, rather than the sourcepackagename. Tests that dealt with the vocabulary returning binary package name titles have been removed, as the vocabulary isn't responsible for that.
Tests
=====
bin/test --vvct dsp_vocabulary
QA
==
With the new dsp vocabulary feature flag enabled, use a package picker (such as the one on the question retargeting widget). The picker entries (in terms of title and package data, but not in terms of other data like affiliation or expanded info) conform to the package examples in the picture above.
Lint
====
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/app/browser/configure.zcml
lib/lp/app/browser/vocabulary.py
lib/lp/registry/vocabularies.py
lib/lp/registry/tests/test_dsp_vocabularies.py
./lib/lp/app/browser/vocabulary.py
336: E251 no spaces around keyword / parameter equals
./lib/lp/registry/vocabularies.py
2136: local variable 'binary_names' is assigned to but never used
The two remaining lint errors can be ignored.
--
https://code.launchpad.net/~jcsackett/launchpad/fix-picker-entry-adapters/+merge/71204
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jcsackett/launchpad/fix-picker-entry-adapters into lp:launchpad.
=== modified file 'lib/lp/app/browser/configure.zcml'
--- lib/lp/app/browser/configure.zcml 2011-08-09 02:06:15 +0000
+++ lib/lp/app/browser/configure.zcml 2011-08-11 13:31:09 +0000
@@ -415,6 +415,10 @@
/>
<adapter
+ factory="lp.app.browser.vocabulary.DistributionSourcePackagePickerEntrySourceAdapter"
+ />
+
+ <adapter
factory="lp.app.browser.vocabulary.ArchivePickerEntrySourceAdapter"
/>
=== modified file 'lib/lp/app/browser/vocabulary.py'
--- lib/lp/app/browser/vocabulary.py 2011-08-09 12:14:42 +0000
+++ lib/lp/app/browser/vocabulary.py 2011-08-11 13:31:09 +0000
@@ -40,10 +40,12 @@
from canonical.launchpad.webapp.vocabulary import IHugeVocabulary
from lp.app.errors import UnexpectedFormData
from lp.code.interfaces.branch import IBranch
+from lp.registry.interfaces.distributionsourcepackage import (
+ IDistributionSourcePackage,
+ )
from lp.registry.interfaces.person import IPerson
from lp.registry.interfaces.sourcepackagename import ISourcePackageName
from lp.registry.model.pillaraffiliation import IHasAffiliation
-from lp.registry.model.sourcepackagename import getSourcePackageDescriptions
from lp.services.features import getFeatureFlag
from lp.soyuz.interfaces.archive import IArchive
@@ -238,11 +240,32 @@
return entries
+@adapter(IDistributionSourcePackage)
+class DistributionSourcePackagePickerEntrySourceAdapter(
+ DefaultPickerEntrySourceAdapter):
+ """Adapts ISourcePackageName to IPickerEntrySource."""
+
+ def getPickerEntries(self, term_values, context_object, **kwarg):
+ """See `IPickerEntrySource`"""
+ entries = (
+ super(DistributionSourcePackagePickerEntrySourceAdapter, self)
+ .getPickerEntries(term_values, context_object, **kwarg))
+ for dsp, picker_entry in izip(term_values, entries):
+ binaries = dsp.publishing_history[0].getBuiltBinaries()
+ binary_names = [binary.binary_package_name for binary in binaries]
+ if binary_names != []:
+ description = ', '.join(binary_names)
+ else:
+ description = 'Not yet built.'
+ picker_entry.description = description
+ return entries
+
+
@adapter(IArchive)
class ArchivePickerEntrySourceAdapter(DefaultPickerEntrySourceAdapter):
"""Adapts IArchive to IPickerEntrySource."""
- def getPickerEntry(self, term_values, context_object, **kwarg):
+ def getPickerEntries(self, term_values, context_object, **kwarg):
"""See `IPickerEntrySource`"""
entries = (
super(ArchivePickerEntrySourceAdapter, self)
=== modified file 'lib/lp/registry/tests/test_dsp_vocabularies.py'
--- lib/lp/registry/tests/test_dsp_vocabularies.py 2011-08-03 05:55:54 +0000
+++ lib/lp/registry/tests/test_dsp_vocabularies.py 2011-08-11 13:31:09 +0000
@@ -122,7 +122,7 @@
term = vocabulary.toTerm(dsp.sourcepackagename)
expected_token = '%s/%s' % (dsp.distribution.name, dsp.name)
self.assertEqual(expected_token, term.token)
- self.assertEqual(dsp.sourcepackagename, term.value)
+ self.assertEqual(dsp, term.value)
def test_toTerm_spn_and_distribution(self):
# The distribution is used with the spn if it is passed.
@@ -132,7 +132,7 @@
term = vocabulary.toTerm(dsp.sourcepackagename, dsp.distribution)
expected_token = '%s/%s' % (dsp.distribution.name, dsp.name)
self.assertEqual(expected_token, term.token)
- self.assertEqual(dsp.sourcepackagename, term.value)
+ self.assertEqual(dsp, term.value)
def test_toTerm_dsp(self):
# The DSP's distribution is used when a DSP is passed.
@@ -142,54 +142,7 @@
term = vocabulary.toTerm(dsp)
expected_token = '%s/%s' % (dsp.distribution.name, dsp.name)
self.assertEqual(expected_token, term.token)
- self.assertEqual(dsp.sourcepackagename, term.value)
-
- def test_toTerm_unbuilt_title(self):
- # The DSP's distribution is used with the spn if it is passed.
- # Published, but unbuilt packages state the case in the term title.
- spph = self.factory.makeSourcePackagePublishingHistory()
- dsp = spph.sourcepackagerelease.distrosourcepackage
- vocabulary = DistributionSourcePackageVocabulary(dsp)
- term = vocabulary.toTerm(dsp)
- expected_title = '%s/%s Not yet built.' % (
- dsp.distribution.name, spph.source_package_name)
- self.assertEqual(expected_title, term.title)
-
- def test_toTerm_built_single_binary_title(self):
- # The binary package name appears in the term's value.
- bpph = self.factory.makeBinaryPackagePublishingHistory()
- spr = bpph.binarypackagerelease.build.source_package_release
- dsp = self.factory.makeDistributionSourcePackage(
- sourcepackagename=spr.sourcepackagename,
- distribution=bpph.distroseries.distribution)
- vocabulary = DistributionSourcePackageVocabulary(dsp.distribution)
- term = vocabulary.toTerm(spr.sourcepackagename)
- expected_title = '%s/%s %s' % (
- dsp.distribution.name, spr.sourcepackagename.name,
- bpph.binary_package_name)
- self.assertEqual(expected_title, term.title)
-
- def test_toTerm_built_multiple_binary_title(self):
- # All of the binary package names appear in the term's value.
- spph = self.factory.makeSourcePackagePublishingHistory()
- spr = spph.sourcepackagerelease
- das = self.factory.makeDistroArchSeries(
- distroseries=spph.distroseries)
- expected_names = []
- for i in xrange(20):
- bpb = self.factory.makeBinaryPackageBuild(
- source_package_release=spr, distroarchseries=das)
- bpr = self.factory.makeBinaryPackageRelease(build=bpb)
- expected_names.append(bpr.name)
- self.factory.makeBinaryPackagePublishingHistory(
- binarypackagerelease=bpr, distroarchseries=das)
- dsp = spr.distrosourcepackage
- vocabulary = DistributionSourcePackageVocabulary(dsp.distribution)
- term = vocabulary.toTerm(spr.sourcepackagename)
- expected_title = '%s/%s %s' % (
- dsp.distribution.name, spph.source_package_name,
- ', '.join(expected_names))
- self.assertEqual(expected_title, term.title)
+ self.assertEqual(dsp, term.value)
def test_getTermByToken_error(self):
# An error is raised if the token does not match a published DSP.
@@ -206,7 +159,7 @@
vocabulary = DistributionSourcePackageVocabulary(dsp.distribution)
token = '%s/%s' % (dsp.distribution.name, dsp.name)
term = vocabulary.getTermByToken(token)
- self.assertEqual(dsp.sourcepackagename, term.value)
+ self.assertEqual(dsp, term.value)
def test_searchForTerms_without_distribution(self):
# An empty result set is return if the vocabulary has no distribution
=== modified file 'lib/lp/registry/vocabularies.py'
--- lib/lp/registry/vocabularies.py 2011-08-02 23:40:08 +0000
+++ lib/lp/registry/vocabularies.py 2011-08-11 13:31:09 +0000
@@ -2128,14 +2128,15 @@
dsp = distribution.getSourcePackage(spn_or_dsp)
try:
token = '%s/%s' % (dsp.distribution.name, dsp.name)
+ summary = '%s (%s)' % (token, dsp.name)
+
+ # We try to get the binaries for the dsp; if this fails, we return
+ # lookup error instead.
binaries = dsp.publishing_history[0].getBuiltBinaries()
binary_names = [binary.binary_package_name for binary in binaries]
- if binary_names != []:
- summary = ', '.join(binary_names)
- else:
- summary = 'Not yet built.'
- summary = token + ' ' + summary
- return SimpleTerm(dsp.sourcepackagename, token, summary)
+
+ return SimpleTerm(dsp, token, summary)
+ #return SimpleTerm(dsp, token, summary)
except (IndexError, AttributeError):
# Either the DSP was None or there is no publishing history.
raise LookupError(distribution, spn_or_dsp)
