launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #04451
[Merge] lp:~allenap/launchpad/selected-differences-vocab-bug-817408 into lp:launchpad
Gavin Panella has proposed merging lp:~allenap/launchpad/selected-differences-vocab-bug-817408 into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #817408 in Launchpad itself: "IDifferencesFormSchema.selected_differences vocabulary is too restrictive"
https://bugs.launchpad.net/launchpad/+bug/817408
For more details, see:
https://code.launchpad.net/~allenap/launchpad/selected-differences-vocab-bug-817408/+merge/70048
This adds a new vocabulary called "DistroSeriesDifferences" which represents all DistroSeriesDifference records for a given derived series. This is then used in place of the hand-crafted vocabulary in IDifferencesFormSchema.
--
https://code.launchpad.net/~allenap/launchpad/selected-differences-vocab-bug-817408/+merge/70048
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~allenap/launchpad/selected-differences-vocab-bug-817408 into lp:launchpad.
=== modified file 'lib/lp/registry/browser/distroseries.py'
--- lib/lp/registry/browser/distroseries.py 2011-07-29 20:05:40 +0000
+++ lib/lp/registry/browser/distroseries.py 2011-08-01 16:34:19 +0000
@@ -792,7 +792,7 @@
selected_differences = List(
title=_('Selected differences'),
- value_type=Choice(vocabulary=SimpleVocabulary([])),
+ value_type=Choice(vocabulary="DistroSeriesDifferences"),
description=_("Select the differences for syncing."),
required=True)
@@ -859,15 +859,6 @@
self.form_fields = (
self.setupPackageFilterRadio() +
self.form_fields)
- check_permission('launchpad.Edit', self.context)
- terms = [
- SimpleTerm(diff, diff.id)
- for diff in self.cached_differences.batch]
- diffs_vocabulary = SimpleVocabulary(terms)
- # XXX: GavinPanella 2011-07-13 bug=809985: Good thing the app servers
- # are running single threaded...
- choice = self.form_fields['selected_differences'].field.value_type
- choice.vocabulary = diffs_vocabulary
def _sync_sources(self, action, data):
"""Synchronise packages from the parent series to this one."""
=== modified file 'lib/lp/registry/tests/test_distroseries_vocabularies.py'
--- lib/lp/registry/tests/test_distroseries_vocabularies.py 2011-07-08 14:09:11 +0000
+++ lib/lp/registry/tests/test_distroseries_vocabularies.py 2011-08-01 16:34:19 +0000
@@ -13,19 +13,29 @@
from pytz import utc
from testtools.matchers import Equals
from zope.component import getUtility
-from zope.schema.interfaces import IVocabularyFactory
+from zope.schema.interfaces import (
+ ITokenizedTerm,
+ IVocabularyFactory,
+ )
from zope.security.proxy import removeSecurityProxy
from canonical.database.sqlbase import flush_database_caches
from canonical.launchpad.webapp.vocabulary import IHugeVocabulary
from canonical.testing.layers import DatabaseFunctionalLayer
from lp.registry.interfaces.distroseries import IDistroSeriesSet
-from lp.registry.vocabularies import DistroSeriesDerivationVocabulary
+from lp.registry.vocabularies import (
+ DistroSeriesDerivationVocabulary,
+ DistroSeriesDifferencesVocabulary,
+ )
from lp.testing import (
StormStatementRecorder,
TestCaseWithFactory,
)
-from lp.testing.matchers import HasQueryCount
+from lp.testing.matchers import (
+ Contains,
+ HasQueryCount,
+ Provides,
+ )
class TestDistroSeriesDerivationVocabulary(TestCaseWithFactory):
@@ -47,9 +57,8 @@
getUtility(IVocabularyFactory, name="DistroSeriesDerivation"),
DistroSeriesDerivationVocabulary)
- def test_interfaces(self):
- # DistroSeriesDerivationVocabulary instances provide
- # IVocabulary and IVocabularyTokenized.
+ def test_interface(self):
+ # DistroSeriesDerivationVocabulary instances provide IHugeVocabulary.
distroseries = self.factory.makeDistroSeries()
vocabulary = DistroSeriesDerivationVocabulary(distroseries)
self.assertProvides(vocabulary, IHugeVocabulary)
@@ -248,3 +257,108 @@
self.assertContentEqual(
expected_distroseries,
observed_distroseries)
+
+
+class TestDistroSeriesDifferencesVocabulary(TestCaseWithFactory):
+ """Tests for `DistroSeriesDifferencesVocabulary`."""
+
+ layer = DatabaseFunctionalLayer
+
+ def test_registration(self):
+ # DistroSeriesDifferencesVocabulary is registered as a named utility
+ # for IVocabularyFactory.
+ self.assertEqual(
+ getUtility(IVocabularyFactory, name="DistroSeriesDifferences"),
+ DistroSeriesDifferencesVocabulary)
+
+ def test_interface(self):
+ # DistroSeriesDifferencesVocabulary instances provide IHugeVocabulary.
+ distroseries = self.factory.makeDistroSeries()
+ vocabulary = DistroSeriesDifferencesVocabulary(distroseries)
+ self.assertProvides(vocabulary, IHugeVocabulary)
+
+ def test_non_derived_distroseries(self):
+ # The vocabulary is empty for a non-derived series.
+ distroseries = self.factory.makeDistroSeries()
+ vocabulary = DistroSeriesDifferencesVocabulary(distroseries)
+ self.assertContentEqual([], vocabulary)
+
+ def test_derived_distroseries(self):
+ # The vocabulary contains all DSDs for a derived series.
+ distroseries = self.factory.makeDistroSeries()
+ dsds = [
+ self.factory.makeDistroSeriesDifference(
+ derived_series=distroseries),
+ self.factory.makeDistroSeriesDifference(
+ derived_series=distroseries),
+ ]
+ vocabulary = DistroSeriesDifferencesVocabulary(distroseries)
+ self.assertContentEqual(
+ dsds, (term.value for term in vocabulary))
+
+ def test_derived_distroseries_not_other_distroseries(self):
+ # The vocabulary contains all DSDs for a derived series and not for
+ # another series.
+ distroseries1 = self.factory.makeDistroSeries()
+ distroseries2 = self.factory.makeDistroSeries()
+ dsds = [
+ self.factory.makeDistroSeriesDifference(
+ derived_series=distroseries1),
+ self.factory.makeDistroSeriesDifference(
+ derived_series=distroseries1),
+ self.factory.makeDistroSeriesDifference(
+ derived_series=distroseries2),
+ self.factory.makeDistroSeriesDifference(
+ derived_series=distroseries2),
+ ]
+ vocabulary = DistroSeriesDifferencesVocabulary(distroseries1)
+ self.assertContentEqual(
+ (dsd for dsd in dsds if dsd.derived_series == distroseries1),
+ (term.value for term in vocabulary))
+
+ def test_contains(self):
+ # The vocabulary can be tested for membership.
+ difference = self.factory.makeDistroSeriesDifference()
+ vocabulary = DistroSeriesDifferencesVocabulary(
+ difference.derived_series)
+ self.assertThat(vocabulary, Contains(difference))
+
+ def test_size(self):
+ # The vocabulary can report its size.
+ difference = self.factory.makeDistroSeriesDifference()
+ vocabulary = DistroSeriesDifferencesVocabulary(
+ difference.derived_series)
+ self.assertEqual(1, len(vocabulary))
+
+ def test_getTerm(self):
+ # A term can be obtained from a given value.
+ difference = self.factory.makeDistroSeriesDifference()
+ vocabulary = DistroSeriesDifferencesVocabulary(
+ difference.derived_series)
+ term = vocabulary.getTerm(difference)
+ self.assertThat(term, Provides(ITokenizedTerm))
+ self.assertEqual(difference, term.value)
+ self.assertEqual(str(difference.id), term.token)
+
+ def test_getTermByToken(self):
+ # A term can be obtained from a given token.
+ difference = self.factory.makeDistroSeriesDifference()
+ vocabulary = DistroSeriesDifferencesVocabulary(
+ difference.derived_series)
+ term = vocabulary.getTermByToken(str(difference.id))
+ self.assertEqual(difference, term.value)
+
+ def test_getTermByToken_not_found(self):
+ # LookupError is raised when the token cannot be found.
+ distroseries = self.factory.makeDistroSeries()
+ difference = self.factory.makeDistroSeriesDifference()
+ vocabulary = DistroSeriesDifferencesVocabulary(distroseries)
+ self.assertRaises(
+ LookupError, vocabulary.getTermByToken, str(difference.id))
+
+ def test_getTermByToken_invalid(self):
+ # LookupError is raised when the token is not valid (i.e. a string
+ # containing only digits).
+ distroseries = self.factory.makeDistroSeries()
+ vocabulary = DistroSeriesDifferencesVocabulary(distroseries)
+ self.assertRaises(LookupError, vocabulary.getTermByToken, "foobar")
=== modified file 'lib/lp/registry/vocabularies.py'
--- lib/lp/registry/vocabularies.py 2011-07-26 19:09:48 +0000
+++ lib/lp/registry/vocabularies.py 2011-08-01 16:34:19 +0000
@@ -33,6 +33,7 @@
'DistributionSourcePackageVocabulary',
'DistributionVocabulary',
'DistroSeriesDerivationVocabulary',
+ 'DistroSeriesDifferencesVocabulary',
'DistroSeriesVocabulary',
'FeaturedProjectVocabulary',
'FilteredDistroSeriesVocabulary',
@@ -167,6 +168,7 @@
from lp.registry.interfaces.sourcepackage import ISourcePackage
from lp.registry.model.distribution import Distribution
from lp.registry.model.distroseries import DistroSeries
+from lp.registry.model.distroseriesdifference import DistroSeriesDifference
from lp.registry.model.distroseriesparent import DistroSeriesParent
from lp.registry.model.featuredproject import FeaturedProject
from lp.registry.model.karma import KarmaCategory
@@ -194,9 +196,7 @@
from lp.soyuz.model.binarypackagename import BinaryPackageName
from lp.soyuz.model.binarypackagerelease import BinaryPackageRelease
from lp.soyuz.model.distroarchseries import DistroArchSeries
-from lp.soyuz.model.publishing import (
- SourcePackagePublishingHistory,
- )
+from lp.soyuz.model.publishing import SourcePackagePublishingHistory
from lp.soyuz.model.sourcepackagerelease import SourcePackageRelease
@@ -1866,6 +1866,75 @@
return self.find_terms(where)
+class DistroSeriesDifferencesVocabulary:
+ """A vocabulary source for differences relating to a series.
+
+ Specifically, all `DistroSeriesDifference`s relating to a derived series.
+ """
+
+ implements(IHugeVocabulary)
+
+ displayname = "Choose a difference"
+ step_title = 'Search'
+
+ def __init__(self, context):
+ """Create a new vocabulary for the context.
+
+ :type context: `IDistroSeries`.
+ """
+ assert IDistroSeries.providedBy(context)
+ self.distroseries = context
+
+ def __len__(self):
+ """See `IIterableVocabulary`."""
+ return self.searchForDifferences().count()
+
+ def __iter__(self):
+ """See `IIterableVocabulary`."""
+ for difference in self.searchForDifferences():
+ yield self.toTerm(difference)
+
+ def __contains__(self, value):
+ """See `IVocabulary`."""
+ return value.derived_series == self.distroseries
+
+ def getTerm(self, value):
+ """See `IVocabulary`."""
+ if value not in self:
+ raise LookupError(value)
+ return self.toTerm(value)
+
+ def getTermByToken(self, token):
+ """See `IVocabularyTokenized`."""
+ if not token.isdigit():
+ raise LookupError(token)
+ difference = IStore(DistroSeriesDifference).get(
+ DistroSeriesDifference, int(token))
+ if difference is None:
+ raise LookupError(token)
+ elif difference.derived_series != self.distroseries:
+ raise LookupError(token)
+ else:
+ return self.toTerm(difference)
+
+ @staticmethod
+ def toTerm(dsd):
+ """Return the term for a `DistroSeriesDifference`."""
+ return SimpleTerm(dsd, dsd.id)
+
+ def searchForTerms(self, query=None):
+ """See `IHugeVocabulary`."""
+ results = self.searchForDifferences()
+ return CountableIterator(results.count(), results, self.toTerm)
+
+ def searchForDifferences(self):
+ """The set of `DistroSeriesDifference`s related to the context.
+
+ :return: `IResultSet` yielding `IDistroSeriesDifference`.
+ """
+ return DistroSeriesDifference.getForDistroSeries(self.distroseries)
+
+
class PillarVocabularyBase(NamedSQLObjectHugeVocabulary):
"""Active `IPillar` objects vocabulary."""
displayname = 'Needs to be overridden'
=== modified file 'lib/lp/registry/vocabularies.zcml'
--- lib/lp/registry/vocabularies.zcml 2011-07-13 01:49:53 +0000
+++ lib/lp/registry/vocabularies.zcml 2011-08-01 16:34:19 +0000
@@ -42,6 +42,18 @@
<securedutility
+ name="DistroSeriesDifferences"
+ component=".vocabularies.DistroSeriesDifferencesVocabulary"
+ provides="zope.schema.interfaces.IVocabularyFactory">
+ <allow interface="zope.schema.interfaces.IVocabularyFactory"/>
+ </securedutility>
+
+ <class class=".vocabularies.DistroSeriesDifferencesVocabulary">
+ <allow interface="canonical.launchpad.webapp.vocabulary.IHugeVocabulary"/>
+ </class>
+
+
+ <securedutility
name="FeaturedProject"
component="lp.registry.vocabularies.FeaturedProjectVocabulary"
provides="zope.schema.interfaces.IVocabularyFactory"
