launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #02880
[Merge] lp:~rvb/launchpad/add-checkbox-blacklisted-localpackagediff into lp:launchpad
Raphaël Victor Badin has proposed merging lp:~rvb/launchpad/add-checkbox-blacklisted-localpackagediff into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
#730711 Add package filter and blacklisted checkbox on the source package differences page
https://bugs.launchpad.net/bugs/730711
For more details, see:
https://code.launchpad.net/~rvb/launchpad/add-checkbox-blacklisted-localpackagediff/+merge/52431
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On the source package difference page (distrib1/series
/+localpackagediffs), add a text search to allow filtering by package
name and a checkbox to be able to also list blacklisted packages.
To test,
{{{
./bin/test -vvc test_series_views test_filter_noform_if_nodifferences
./bin/test -vvc test_series_views test_filter_form_if_differences
./bin/test -vvc test_series_views test_batch_filtered
./bin/test -vvc test_series_views test_batch_blacklisted
}}}
No lint.
Raphaël
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
iQIcBAEBAgAGBQJNdQhaAAoJEMC8revFINFcgxwP/imYL9Sa+taOsx49SGtTA80H
26xExZwLaXRMo8lXpAzZwar1Wvar1zVnPHCj539wYqnD0hq5oAVsGg+zgmeWxLVi
ZFh77XfrpkFMBSpW+IT39hWNK7xb9JwAJLaeglQAzr+RpcXWV6GR+x3fhMNxYugc
BJNkHsXKdfNEAArSO7QdM+iCKiEq6SHNjk5SwHjFtjkZzCpz1Ye629EXMXcnJp34
NeMCRI6tSGGgw63CPDZDeiillktvMxsCNYImKpcaMEHSNk0exV+7+MqjzuphiZex
RwbtWrx9vN9WlUhyHr9G8BT2Z9bfdDDjoaQtN9gYlX2K6EPCbSrsp9oQJDGfFevd
lqX+CP+Qn1ytN7kalgtYY4+Q/9faaNxUQRiIgRJLRlfc4S/m36xaxyNkjpBzS8tQ
F8LnhZeQA2VpBgIIUzMJkCsr1aBXgok67FN1n12eEuNikeXZLatAzgnyjX486zZj
eD2kDO3AcJfeCjgjj0QZ1yzEIfcKna1w1jccd2dl9AyvCuklpNTKfJMB/4NjyaHO
XnDxo5kIKQvYeESKjPs7+tQ7/z2yLWdCTvkiVxOLXdsXUNNgzyGQ6Xf+oxTLaEtX
poH1tRAmcTtr/XKHmtiyOk5+d+jmrFH5+mJGsGEcqkK62AgE6grKPWpwU7oKrPf3
s6nI3tHn3GiubsOSw9P2
=DJej
-----END PGP SIGNATURE-----
--
https://code.launchpad.net/~rvb/launchpad/add-checkbox-blacklisted-localpackagediff/+merge/52431
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/launchpad/add-checkbox-blacklisted-localpackagediff into lp:launchpad.
=== modified file 'lib/lp/registry/browser/distroseries.py'
--- lib/lp/registry/browser/distroseries.py 2011-02-03 10:35:36 +0000
+++ lib/lp/registry/browser/distroseries.py 2011-03-07 16:36:57 +0000
@@ -24,8 +24,10 @@
from zope.interface import Interface
from zope.lifecycleevent import ObjectCreatedEvent
from zope.schema import (
+ Bool,
Choice,
List,
+ TextLine,
)
from zope.schema.vocabulary import (
SimpleTerm,
@@ -77,6 +79,7 @@
StructuralSubscriptionTargetTraversalMixin,
)
from lp.registry.browser import MilestoneOverlayMixin
+from lp.registry.enum import DistroSeriesDifferenceStatus
from lp.registry.interfaces.distroseries import IDistroSeries
from lp.registry.interfaces.distroseriesdifference import (
IDistroSeriesDifferenceSource,
@@ -544,6 +547,73 @@
return navigator
+class DistroSeriesLocalDifferencesListFilter(Interface):
+ """The interface used as the schema for the package filtering form."""
+ name_filter = TextLine(
+ title=_("Package name contains"), required=False)
+
+ include_blacklisted_filter = Bool(
+ title=_("include blacklisted packages"),
+ required=False, default=False)
+
+
+class DistroSeriesLocalDifferencesListView(LaunchpadFormView):
+ """A Form view for filtering and batching source packages."""
+
+ schema = DistroSeriesLocalDifferencesListFilter
+
+ @property
+ def specified_name_filter(self):
+ """Return the specified name filter if one was specified """
+ requested_name_filter = self.request.query_string_params.get(
+ 'field.name_filter')
+
+ if requested_name_filter and requested_name_filter[0]:
+ return requested_name_filter[0]
+ else:
+ return None
+
+ @property
+ def specified_include_blacklisted_filter(self):
+ """Return the specified name filter if one was specified """
+ 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]
+ else:
+ return None
+
+ @cachedproperty
+ def cached_differences(self):
+ """Return a batch navigator of filtered results."""
+ utility = getUtility(IDistroSeriesDifferenceSource)
+ if self.specified_include_blacklisted_filter:
+ status=(
+ DistroSeriesDifferenceStatus.NEEDS_ATTENTION,
+ DistroSeriesDifferenceStatus.BLACKLISTED_CURRENT)
+ else:
+ status=(
+ DistroSeriesDifferenceStatus.NEEDS_ATTENTION,)
+ differences = utility.getForDistroSeries(
+ self.context,
+ source_package_name_filter=self.specified_name_filter,
+ status=status)
+ return BatchNavigator(differences, self.request)
+
+ @cachedproperty
+ def has_differences(self):
+ """Whether or not differences between this derived series and
+ it's parent exist."""
+ utility = getUtility(IDistroSeriesDifferenceSource)
+ differences = utility.getForDistroSeries(
+ self.context,
+ status=(
+ DistroSeriesDifferenceStatus.NEEDS_ATTENTION,
+ DistroSeriesDifferenceStatus.BLACKLISTED_CURRENT))
+ return bool(differences.count())
+
+
class IDifferencesFormSchema(Interface):
selected_differences = List(
title=_('Selected differences'),
@@ -552,7 +622,8 @@
required=True)
-class DistroSeriesLocalDifferences(LaunchpadFormView):
+class DistroSeriesLocalDifferences(DistroSeriesLocalDifferencesListView,
+ LaunchpadFormView):
"""Present differences between a derived series and its parent."""
schema = IDifferencesFormSchema
custom_widget('selected_differences', LabeledMultiCheckBoxWidget)
@@ -582,13 +653,6 @@
self.context.parent_series.displayname,
))
- @cachedproperty
- def cached_differences(self):
- """Return a batch navigator of potentially filtered results."""
- utility = getUtility(IDistroSeriesDifferenceSource)
- differences = utility.getForDistroSeries(self.context)
- return BatchNavigator(differences, self.request)
-
def setUpFields(self):
"""Add the selected differences field.
@@ -606,6 +670,11 @@
choice = self.form_fields['selected_differences'].field.value_type
choice.vocabulary = diffs_vocabulary
+ @action(_("Update"), name="update")
+ def update_action(self, action, data):
+ """Simply re-issue the form with the new values."""
+ pass
+
@action(_("Sync Sources"), name="sync", validator='validate_sync',
condition='canPerformSync')
def sync_sources(self, action, data):
=== modified file 'lib/lp/registry/browser/tests/test_series_views.py'
--- lib/lp/registry/browser/tests/test_series_views.py 2010-10-29 19:28:14 +0000
+++ lib/lp/registry/browser/tests/test_series_views.py 2011-03-07 16:36:57 +0000
@@ -12,9 +12,11 @@
from canonical.config import config
from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
+from canonical.launchpad.testing.pages import find_tag_by_id
from canonical.launchpad.webapp.batching import BatchNavigator
from canonical.launchpad.webapp.publisher import canonical_url
from canonical.testing.layers import (
+ DatabaseFunctionalLayer,
LaunchpadZopelessLayer,
LaunchpadFunctionalLayer,
)
@@ -33,6 +35,7 @@
)
from lp.testing import (
TestCaseWithFactory,
+ login_person,
person_logged_in,
)
from lp.testing.views import create_initialized_view
@@ -85,6 +88,52 @@
test_case.addCleanup(reset_per_thread_features)
+class DistroSeriesLocalPackageDiffsPageTestCase(TestCaseWithFactory):
+ """Test the distroseries +localpackagediffs page."""
+
+ layer = DatabaseFunctionalLayer
+
+ def setUp(self):
+ super(DistroSeriesLocalPackageDiffsPageTestCase,
+ self).setUp('foo.bar@xxxxxxxxxxxxx')
+ set_derived_series_ui_feature_flag(self)
+ self.simple_user = self.factory.makePerson()
+
+ def test_filter_form_if_differences(self):
+ # Test that the page includes the filter form if differences
+ # are present
+ login_person(self.simple_user)
+ derived_series = self.factory.makeDistroSeries(
+ name='derilucid', parent_series=self.factory.makeDistroSeries(
+ name='lucid'))
+ current_difference = self.factory.makeDistroSeriesDifference(
+ derived_series=derived_series)
+
+ view = create_initialized_view(
+ derived_series, '+localpackagediffs', principal=self.simple_user)
+
+ self.assertIsNot(
+ None,
+ find_tag_by_id(view(), 'distroseries-localdiff-search-filter'),
+ "Form filter should be show when there is differences.")
+
+ def test_filter_noform_if_nodifferences(self):
+ # Test that the page doesn't includes the filter form if no
+ # differences are present
+ login_person(self.simple_user)
+ derived_series = self.factory.makeDistroSeries(
+ name='derilucid', parent_series=self.factory.makeDistroSeries(
+ name='lucid'))
+
+ view = create_initialized_view(
+ derived_series, '+localpackagediffs', principal=self.simple_user)
+
+ self.assertIs(
+ None,
+ find_tag_by_id(view(), 'distroseries-localdiff-search-filter'),
+ "Form filter should not be show when there is no differences.")
+
+
class DistroSeriesLocalPackageDiffsTestCase(TestCaseWithFactory):
"""Test the distroseries +localpackagediffs view."""
@@ -221,6 +270,56 @@
layer = LaunchpadFunctionalLayer
+ def test_batch_filtered(self):
+ # The name_filter parameter allows to filter packages by name.
+ 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")
+
+ filtered_view = create_initialized_view(
+ derived_series,
+ '+localpackagediffs',
+ query_string='field.name_filter=my-src')
+ unfiltered_view = create_initialized_view(
+ derived_series,
+ '+localpackagediffs')
+
+ self.assertContentEqual(
+ [diff1], filtered_view.cached_differences.batch)
+ self.assertContentEqual(
+ [diff2, diff1], unfiltered_view.cached_differences.batch)
+
+ def test_batch_blacklisted(self):
+ # The include_blacklisted_filter parameter allows to list
+ # blacklisted packages.
+ set_derived_series_ui_feature_flag(self)
+ derived_series = self.factory.makeDistroSeries(
+ name='derilucid', parent_series=self.factory.makeDistroSeries(
+ name='lucid'))
+ blacklisted_diff = self.factory.makeDistroSeriesDifference(
+ derived_series=derived_series,
+ status=DistroSeriesDifferenceStatus.BLACKLISTED_CURRENT)
+
+ blacklisted_view = create_initialized_view(
+ derived_series,
+ '+localpackagediffs',
+ query_string='field.include_blacklisted_filter=on')
+ unblacklisted_view = create_initialized_view(
+ derived_series,
+ '+localpackagediffs')
+
+ self.assertContentEqual(
+ [blacklisted_diff], 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 2010-10-07 14:59:21 +0000
+++ lib/lp/registry/interfaces/distroseriesdifference.py 2011-03-07 16:36:57 +0000
@@ -222,6 +222,7 @@
def getForDistroSeries(
distro_series,
difference_type=DistroSeriesDifferenceType.DIFFERENT_VERSIONS,
+ source_package_name_filter=None,
status=None):
"""Return differences for the derived distro series.
@@ -231,6 +232,8 @@
:param difference_type: The type of difference to include in the
results.
:type difference_type: `DistroSeriesDifferenceType`.
+ :param source_package_name_filter: Package source name filter.
+ :type source_package_name_filter: unicode.
:param status: Only differences matching the status(es) will be
included.
:type status: `DistroSeriesDifferenceStatus`.
=== modified file 'lib/lp/registry/model/distroseriesdifference.py'
--- lib/lp/registry/model/distroseriesdifference.py 2011-03-04 05:54:40 +0000
+++ lib/lp/registry/model/distroseriesdifference.py 2011-03-07 16:36:57 +0000
@@ -11,7 +11,9 @@
from lazr.enum import DBItem
from storm.expr import Desc
+
from storm.locals import (
+ And,
Int,
Reference,
Storm,
@@ -114,6 +116,7 @@
def getForDistroSeries(
distro_series,
difference_type=DistroSeriesDifferenceType.DIFFERENT_VERSIONS,
+ source_package_name_filter=None,
status=None):
"""See `IDistroSeriesDifferenceSource`."""
if status is None:
@@ -123,11 +126,21 @@
elif isinstance(status, DBItem):
status = (status, )
- return IStore(DistroSeriesDifference).find(
- DistroSeriesDifference,
+ conditions = [
DistroSeriesDifference.derived_series == distro_series,
DistroSeriesDifference.difference_type == difference_type,
- DistroSeriesDifference.status.is_in(status))
+ DistroSeriesDifference.status.is_in(status),
+ ]
+ if source_package_name_filter:
+ conditions.extend([
+ DistroSeriesDifference.source_package_name ==
+ SourcePackageName.id,
+ SourcePackageName.name.like(
+ u'%%%s%%' %source_package_name_filter)])
+
+ return IStore(DistroSeriesDifference).find(
+ DistroSeriesDifference,
+ And(*conditions))
@staticmethod
def getByDistroSeriesAndName(distro_series, source_package_name):
=== modified file 'lib/lp/registry/templates/distroseries-localdifferences.pt'
--- lib/lp/registry/templates/distroseries-localdifferences.pt 2011-02-21 04:17:09 +0000
+++ lib/lp/registry/templates/distroseries-localdifferences.pt 2011-03-07 16:36:57 +0000
@@ -27,6 +27,13 @@
more about syncing from the parent series</a>).
</p>
+ <tal:distroseries_localdiff_search_form
+ tal:condition="view/has_differences">
+ <metal:package_filter_form
+ use-macro="context/@@+macros/distroseries-localdiff-search-form" />
+ </tal:distroseries_localdiff_search_form>
+
+
<div metal:use-macro="context/@@launchpad_form/form">
<div tal:condition="differences/batch" metal:fill-slot="widgets">
=== modified file 'lib/lp/registry/templates/packagesearch-macros.pt'
--- lib/lp/registry/templates/packagesearch-macros.pt 2009-07-17 17:59:07 +0000
+++ lib/lp/registry/templates/packagesearch-macros.pt 2011-03-07 16:36:57 +0000
@@ -53,4 +53,41 @@
</div>
</metal:search_results>
+<metal:distroseries_localdiff_search_form
+ define-macro="distroseries-localdiff-search-form">
+ <tal:comment replace="nothing">
+ Present the filtering form used on a few archive pages.
+ </tal:comment>
+ <form id="distroseries-localdiff-search-filter"
+ class="distroseries-localdiff-search-filter"
+ action="" method="GET">
+ <table>
+ <tbody>
+ <tr>
+ <td>
+ <label for="name">Show packages with names matching:</label>
+ </td>
+ <td>
+ <input id="field.name_filter" title="Package Name" size="20"
+ type="text" name="field.name_filter"
+ tal:attributes="value request/field.name_filter|nothing"/>
+ <input type="submit" value="Filter" />
+ </td>
+ </tr>
+ <tr>
+ <td>
+ </td>
+ <td>
+ <input
+ id="field.include_blacklisted_filter"
+ type="checkbox" name="field.include_blacklisted_filter"
+ tal:attributes="checked request/field.include_blacklisted_filter|nothing"/>
+ <label for="name">include blacklisted packages</label>
+ </td>
+ </tr>
+ </tbody>
+ </table>
+ </form>
+
+</metal:distroseries_localdiff_search_form>
</tal:root>
