launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #03001
[Merge] lp:~rvb/launchpad/db-dds-diffpage-form into lp:launchpad/db-devel
Raphaël Victor Badin has proposed merging lp:~rvb/launchpad/db-dds-diffpage-form into lp:launchpad/db-devel.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #737165 in Launchpad itself: "Add search option for higher versions of local packages"
https://bugs.launchpad.net/launchpad/+bug/737165
For more details, see:
https://code.launchpad.net/~rvb/launchpad/db-dds-diffpage-form/+merge/53913
== Summary ==
This branch allows to search for higher versions of local packages in the derived distroseries difference page.
This branch targets db-devel because it also changes the database types for the columns storing debian package versions in the distroseriesdifference object.
== Implementation ==
- Change the form to use a radio button (instead of checkbox) to select the kind of differences to display.
- Change source_version, parent_source_version and base_version in the distroseriesdifference to debversion type.
== Tests ==
{{{
./bin/test -cvv test_series_views test_batch_blacklisted_differences_with_higher_version
./bin/test -cvv test_series_views test_batch_unfiltered
./bin/test -cvv test_series_views test_batch_differences_packages
}}}
== Demo and Q/A ==
Test form:
https://server/deribuntu/dangerous/+localpackagediffs
--
https://code.launchpad.net/~rvb/launchpad/db-dds-diffpage-form/+merge/53913
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/launchpad/db-dds-diffpage-form into lp:launchpad/db-devel.
=== added file 'database/schema/patch-2208-99-0.sql'
--- database/schema/patch-2208-99-0.sql 1970-01-01 00:00:00 +0000
+++ database/schema/patch-2208-99-0.sql 2011-03-17 21:31:32 +0000
@@ -0,0 +1,26 @@
+-- Copyright 2011 Canonical Ltd. This software is licensed under the
+-- GNU Affero General Public License version 3 (see the file LICENSE).
+
+SET client_min_messages=ERROR;
+
+-- Convert DistroSeriesDifference source_version, parent_source_version,
+-- and base_version types to debversion.
+
+-- Change types.
+ALTER TABLE DistroSeriesDifference
+ ALTER COLUMN source_version TYPE debversion;
+ALTER TABLE DistroSeriesDifference
+ ALTER COLUMN parent_source_version TYPE debversion;
+ALTER TABLE DistroSeriesDifference
+ ALTER COLUMN base_version TYPE debversion;
+
+-- Create indexes.
+CREATE INDEX SourcePackageRelease__source_version__idx
+ ON DistroSeriesDifference(source_version);
+CREATE INDEX BinaryPackageRelease__parent_source_version__idx
+ ON DistroSeriesDifference(parent_source_version);
+CREATE INDEX BinaryPackageRelease__base_version__idx
+ ON DistroSeriesDifference(base_version);
+
+
+INSERT INTO LaunchpadDatabaseRevision VALUES (2208, 99, 0);
=== modified file 'lib/lp/registry/browser/distroseries.py'
--- lib/lp/registry/browser/distroseries.py 2011-03-08 11:56:40 +0000
+++ lib/lp/registry/browser/distroseries.py 2011-03-17 21:31:32 +0000
@@ -24,7 +24,6 @@
from zope.interface import Interface
from zope.lifecycleevent import ObjectCreatedEvent
from zope.schema import (
- Bool,
Choice,
List,
TextLine,
@@ -69,6 +68,7 @@
from lp.app.widgets.itemswidgets import (
LabeledMultiCheckBoxWidget,
LaunchpadDropdownWidget,
+ LaunchpadRadioWidget,
)
from lp.blueprints.browser.specificationtarget import (
HasSpecificationsMenuMixin,
@@ -532,6 +532,23 @@
return navigator
+# A simple vocabulary for package filtering on the source package
+# differences page
+NON_BLACKLISTED = 'non-blacklisted'
+BLACKLISTED = 'blacklisted'
+HIGHER_VERSION_THAN_PARENT = 'higher-than-parent'
+
+DEFAULT_PACKAGE_TYPE = NON_BLACKLISTED
+
+PACKAGE_TYPE_VOCABULARY = SimpleVocabulary((
+ SimpleTerm(NON_BLACKLISTED, NON_BLACKLISTED, 'Non blacklisted packages'),
+ SimpleTerm(BLACKLISTED, BLACKLISTED, 'Blacklisted packages'),
+ SimpleTerm(
+ HIGHER_VERSION_THAN_PARENT,
+ HIGHER_VERSION_THAN_PARENT,
+ 'Blacklisted packages with a higher version than in the parent')))
+
+
class DistroSeriesNeedsPackagesView(LaunchpadView):
"""A View to show series package to upstream package relationships."""
@@ -551,9 +568,10 @@
name_filter = TextLine(
title=_("Package name contains"), required=False)
- include_blacklisted_filter = Bool(
- title=_("include blacklisted packages"),
- required=False, default=False)
+ package_type = Choice(
+ vocabulary=PACKAGE_TYPE_VOCABULARY,
+ default=DEFAULT_PACKAGE_TYPE,
+ required=True)
selected_differences = List(
title=_('Selected differences'),
@@ -565,8 +583,9 @@
class DistroSeriesLocalDifferences(LaunchpadFormView):
"""Present differences between a derived series and its parent."""
schema = IDifferencesFormSchema
- field_names = ['selected_differences']
+ field_names = ['selected_differences', 'package_type']
custom_widget('selected_differences', LabeledMultiCheckBoxWidget)
+ custom_widget('package_type', LaunchpadRadioWidget)
page_title = 'Local package differences'
@@ -582,6 +601,7 @@
self.context.parent_series.displayname,
self.context.displayname,
))
+
super(DistroSeriesLocalDifferences, self).initialize()
@property
@@ -662,33 +682,41 @@
return None
@property
- def specified_include_blacklisted_filter(self):
- """If specified, return the 'blacklisted' filter from the GET form
+ def specified_package_type(self):
+ """If specified, return the package type filter from the GET form
data.
"""
- include_blacklisted_filter = self.request.query_string_params.get(
- 'field.include_blacklisted_filter')
-
- if include_blacklisted_filter and include_blacklisted_filter[0]:
- return include_blacklisted_filter[0]
+ package_type = self.request.query_string_params.get(
+ 'field.package_type')
+ if package_type and package_type[0]:
+ return package_type[0]
else:
- return None
+ return DEFAULT_PACKAGE_TYPE
@cachedproperty
def cached_differences(self):
"""Return a batch navigator of filtered results."""
- if self.specified_include_blacklisted_filter:
- status=(
- DistroSeriesDifferenceStatus.NEEDS_ATTENTION,
- DistroSeriesDifferenceStatus.BLACKLISTED_CURRENT)
- else:
+ if self.specified_package_type == NON_BLACKLISTED:
status=(
DistroSeriesDifferenceStatus.NEEDS_ATTENTION,)
+ child_version_higher = False
+ elif self.specified_package_type == BLACKLISTED:
+ status=(
+ DistroSeriesDifferenceStatus.BLACKLISTED_CURRENT)
+ child_version_higher = False
+ elif self.specified_package_type == HIGHER_VERSION_THAN_PARENT:
+ status=(
+ DistroSeriesDifferenceStatus.BLACKLISTED_CURRENT)
+ child_version_higher = True
+ else:
+ raise AssertionError('specified_package_type unknown')
+
differences = getUtility(
IDistroSeriesDifferenceSource).getForDistroSeries(
self.context,
source_package_name_filter=self.specified_name_filter,
- status=status)
+ status=status,
+ child_version_higher=child_version_higher)
return BatchNavigator(differences, self.request)
@cachedproperty
=== modified file 'lib/lp/registry/browser/tests/test_series_views.py'
--- lib/lp/registry/browser/tests/test_series_views.py 2011-03-07 19:53:13 +0000
+++ lib/lp/registry/browser/tests/test_series_views.py 2011-03-17 21:31:32 +0000
@@ -38,6 +38,11 @@
login_person,
person_logged_in,
)
+from lp.registry.browser.distroseries import (
+ BLACKLISTED,
+ NON_BLACKLISTED,
+ HIGHER_VERSION_THAN_PARENT,
+ )
from lp.testing.views import create_initialized_view
@@ -296,9 +301,38 @@
self.assertContentEqual(
[diff2, diff1], unfiltered_view.cached_differences.batch)
- def test_batch_blacklisted(self):
- # The include_blacklisted_filter parameter allows to list
- # blacklisted packages.
+ def test_batch_unfiltered(self):
+ # The default filter is all non blacklisted differences.
+ set_derived_series_ui_feature_flag(self)
+ derived_series = self.factory.makeDistroSeries(
+ name='derilucid', parent_series=self.factory.makeDistroSeries(
+ name='lucid'))
+ diff1 = self.factory.makeDistroSeriesDifference(
+ derived_series=derived_series,
+ source_package_name_str="my-src-package")
+ diff2 = self.factory.makeDistroSeriesDifference(
+ derived_series=derived_series,
+ source_package_name_str="my-second-src-package")
+ blacklisted_diff = self.factory.makeDistroSeriesDifference(
+ derived_series=derived_series,
+ status=DistroSeriesDifferenceStatus.BLACKLISTED_CURRENT)
+
+ filtered_view = create_initialized_view(
+ derived_series,
+ '+localpackagediffs',
+ query_string='field.package_type=%s' %NON_BLACKLISTED)
+ filtered_view2 = create_initialized_view(
+ derived_series,
+ '+localpackagediffs')
+
+ self.assertContentEqual(
+ [diff2, diff1], filtered_view.cached_differences.batch)
+ self.assertContentEqual(
+ [diff2, diff1], filtered_view2.cached_differences.batch)
+
+ def test_batch_differences_packages(self):
+ # field.package_type parameter allows to list only
+ # blacklisted differences.
set_derived_series_ui_feature_flag(self)
derived_series = self.factory.makeDistroSeries(
name='derilucid', parent_series=self.factory.makeDistroSeries(
@@ -310,7 +344,7 @@
blacklisted_view = create_initialized_view(
derived_series,
'+localpackagediffs',
- query_string='field.include_blacklisted_filter=on')
+ query_string='field.package_type=%s' %BLACKLISTED)
unblacklisted_view = create_initialized_view(
derived_series,
'+localpackagediffs')
@@ -320,6 +354,36 @@
self.assertContentEqual(
[], unblacklisted_view.cached_differences.batch)
+ def test_batch_blacklisted_differences_with_higher_version(self):
+ # field.package_type parameter allows to list only
+ # blacklisted differences with a child's version higher than parent's.
+ set_derived_series_ui_feature_flag(self)
+ derived_series = self.factory.makeDistroSeries(
+ name='derilucid', parent_series=self.factory.makeDistroSeries(
+ name='lucid'))
+ blacklisted_diff_higher = self.factory.makeDistroSeriesDifference(
+ derived_series=derived_series,
+ status=DistroSeriesDifferenceStatus.BLACKLISTED_CURRENT,
+ versions={'base': '1.1', 'parent': '1.3', 'derived': '1.10'})
+ blacklisted_diff_not_higher = self.factory.makeDistroSeriesDifference(
+ derived_series=derived_series,
+ status=DistroSeriesDifferenceStatus.BLACKLISTED_CURRENT,
+ versions={'base': '1.1', 'parent': '1.12', 'derived': '1.10'})
+
+ blacklisted_view = create_initialized_view(
+ derived_series,
+ '+localpackagediffs',
+ query_string='field.package_type=%s' %HIGHER_VERSION_THAN_PARENT)
+ unblacklisted_view = create_initialized_view(
+ derived_series,
+ '+localpackagediffs')
+
+ self.assertContentEqual(
+ [blacklisted_diff_higher],
+ blacklisted_view.cached_differences.batch)
+ self.assertContentEqual(
+ [], unblacklisted_view.cached_differences.batch)
+
def test_canPerformSync_non_editor(self):
# Non-editors do not see options to sync.
derived_series = self.factory.makeDistroSeries(
=== modified file 'lib/lp/registry/interfaces/distroseriesdifference.py'
--- lib/lp/registry/interfaces/distroseriesdifference.py 2011-03-15 18:38:51 +0000
+++ lib/lp/registry/interfaces/distroseriesdifference.py 2011-03-17 21:31:32 +0000
@@ -1,4 +1,4 @@
-# Copyright 2010 Canonical Ltd. This software is licensed under the
+# Copyright 2010-2011 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Interface classes for a difference between two distribution series."""
@@ -38,8 +38,8 @@
)
from lp.registry.interfaces.distroseries import IDistroSeries
from lp.registry.interfaces.person import IPerson
+from lp.registry.interfaces.role import IHasOwner
from lp.registry.interfaces.sourcepackagename import ISourcePackageName
-from lp.registry.interfaces.role import IHasOwner
from lp.soyuz.interfaces.packagediff import IPackageDiff
from lp.soyuz.interfaces.publishing import ISourcePackagePublishingHistory
@@ -224,7 +224,8 @@
distro_series,
difference_type=DistroSeriesDifferenceType.DIFFERENT_VERSIONS,
source_package_name_filter=None,
- status=None):
+ status=None,
+ child_version_higher=False):
"""Return differences for the derived distro series.
:param distro_series: The derived distribution series which is to be
@@ -238,6 +239,9 @@
:param status: Only differences matching the status(es) will be
included.
:type status: `DistroSeriesDifferenceStatus`.
+ :param child_version_higher: Only differences for which the child's
+ version is higher than the parent's version will be included.
+ :type child_version_higher: bool.
:return: A result set of differences.
"""
=== modified file 'lib/lp/registry/model/distroseriesdifference.py'
--- lib/lp/registry/model/distroseriesdifference.py 2011-03-15 18:38:51 +0000
+++ lib/lp/registry/model/distroseriesdifference.py 2011-03-17 21:31:32 +0000
@@ -11,14 +11,13 @@
from debian.changelog import Changelog
from lazr.enum import DBItem
+from sqlobject import StringCol
from storm.expr import Desc
-
from storm.locals import (
And,
Int,
Reference,
Storm,
- Unicode,
)
from zope.component import getUtility
from zope.interface import (
@@ -88,10 +87,10 @@
enum=DistroSeriesDifferenceStatus)
difference_type = DBEnum(name='difference_type', allow_none=False,
enum=DistroSeriesDifferenceType)
- source_version = Unicode(name='source_version', allow_none=True)
- parent_source_version = Unicode(name='parent_source_version',
- allow_none=True)
- base_version = Unicode(name='base_version', allow_none=True)
+ source_version = StringCol(dbName='source_version', notNull=False)
+ parent_source_version = StringCol(dbName='parent_source_version',
+ notNull=False)
+ base_version = StringCol(dbName='base_version', notNull=False)
@staticmethod
def new(derived_series, source_package_name):
@@ -117,7 +116,8 @@
distro_series,
difference_type=DistroSeriesDifferenceType.DIFFERENT_VERSIONS,
source_package_name_filter=None,
- status=None):
+ status=None,
+ child_version_higher=False):
"""See `IDistroSeriesDifferenceSource`."""
if status is None:
status = (
@@ -131,12 +131,18 @@
DistroSeriesDifference.difference_type == difference_type,
DistroSeriesDifference.status.is_in(status),
]
+
if source_package_name_filter:
conditions.extend([
DistroSeriesDifference.source_package_name ==
SourcePackageName.id,
SourcePackageName.name == source_package_name_filter])
+ if child_version_higher:
+ conditions.extend([
+ DistroSeriesDifference.source_version >
+ DistroSeriesDifference.parent_source_version])
+
return IStore(DistroSeriesDifference).find(
DistroSeriesDifference,
And(*conditions))
=== modified file 'lib/lp/registry/templates/distroseries-localdifferences.pt'
--- lib/lp/registry/templates/distroseries-localdifferences.pt 2011-03-07 13:43:29 +0000
+++ lib/lp/registry/templates/distroseries-localdifferences.pt 2011-03-17 21:31:32 +0000
@@ -6,6 +6,15 @@
metal:use-macro="view/macro:page/main_only"
i18n:domain="launchpad">
<body>
+
+ <metal:block fill-slot="head_epilogue">
+ <style type="text/css">
+ .distroseries-localdiff-search-filter input[type="radio"] {
+ margin-left: 0;
+ }
+ </style>
+ </metal:block>
+
<tal:heading metal:fill-slot="heading">
<h1 tal:content="view/label">Package differences between ...</h1>
</tal:heading>
=== modified file 'lib/lp/registry/templates/packagesearch-macros.pt'
--- lib/lp/registry/templates/packagesearch-macros.pt 2011-03-09 23:03:28 +0000
+++ lib/lp/registry/templates/packagesearch-macros.pt 2011-03-17 21:31:32 +0000
@@ -71,12 +71,9 @@
type="text" name="field.name_filter"
tal:attributes="value request/field.name_filter|nothing"/>
<input type="submit" value="Filter" />
- <br />
- <input
- id="field.include_blacklisted_filter"
- type="checkbox" name="field.include_blacklisted_filter"
- tal:attributes="checked request/field.include_blacklisted_filter|nothing"/>
- <label for="field.include_blacklisted_filter">include blacklisted packages</label>
+ <br />
+ <tal:package_type
+ replace="structure view/widgets/package_type" />
</div>
<div class="clearfix"></div>
</form>
Follow ups
