launchpad-reviewers team mailing list archive

[Raphaël Victor Badin <raphael.badin@xxxxxxxxxxxxx>]

Raphaël Victor Badin has proposed merging lp:~rvb/launchpad/bug-782210 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #782210 in Launchpad itself: "Derived Distros pages should allow searching across *all* available differences"
  https://bugs.launchpad.net/launchpad/+bug/782210

For more details, see:
https://code.launchpad.net/~rvb/launchpad/bug-782210/+merge/69651

This branch removes the option to search among ignored packages on the +localpackagediffs page and adds a new option to search for packages without specifying a status (search in 'All packages').

Drive-by fixes:
- cleanup the formatting for getForDistroSeries
- fix the label of the search form ("Show packages with names matching:" => "Show packages with names or packagesets matching:").

= Tests =

./bin/test -vvc test_distroseries test_batch_all_packages

= QA =

https://dogfood.launchpad.net/ubuntu/oneiric/+localpackagediffs
Make sure this page on DF displays the "All packages" option and that search for a package with this option fetches blacklist as well as non blacklisted packages.

-- 
https://code.launchpad.net/~rvb/launchpad/bug-782210/+merge/69651
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/launchpad/bug-782210 into lp:launchpad.

=== modified file 'lib/lp/registry/browser/distroseries.py'
--- lib/lp/registry/browser/distroseries.py	2011-07-20 16:51:49 +0000
+++ lib/lp/registry/browser/distroseries.py	2011-07-28 13:47:52 +0000
@@ -747,26 +747,26 @@
 
 # A helper to create package filtering radio button vocabulary.
 NON_IGNORED = 'non-ignored'
-IGNORED = 'ignored'
 HIGHER_VERSION_THAN_PARENT = 'higher-than-parent'
 RESOLVED = 'resolved'
+ALL = 'all'
 
 DEFAULT_PACKAGE_TYPE = NON_IGNORED
 
 
 def make_package_type_vocabulary(parent_name, higher_version_option=False):
     voc = [
-        SimpleTerm(
-            NON_IGNORED, NON_IGNORED, 'Non ignored packages'),
-        SimpleTerm(IGNORED, IGNORED, 'Ignored packages'),
-        SimpleTerm(RESOLVED, RESOLVED, "Resolved package differences")]
+        SimpleTerm(NON_IGNORED, NON_IGNORED, 'Non ignored packages'),
+        SimpleTerm(RESOLVED, RESOLVED, "Resolved package differences"),
+        SimpleTerm(ALL, ALL, 'All packages'),
+        ]
     if higher_version_option:
         higher_term = SimpleTerm(
             HIGHER_VERSION_THAN_PARENT,
             HIGHER_VERSION_THAN_PARENT,
             "Ignored packages with a higher version than in %s"
                 % parent_name)
-        voc.insert(2, higher_term)
+        voc.insert(1, higher_term)
     return SimpleVocabulary(tuple(voc))
 
 
@@ -1058,10 +1058,10 @@
         package_type_dsd_status = {
             NON_IGNORED: (
                 DistroSeriesDifferenceStatus.NEEDS_ATTENTION,),
-            IGNORED: DistroSeriesDifferenceStatus.BLACKLISTED_CURRENT,
             HIGHER_VERSION_THAN_PARENT: (
                 DistroSeriesDifferenceStatus.BLACKLISTED_CURRENT),
             RESOLVED: DistroSeriesDifferenceStatus.RESOLVED,
+            ALL: DistroSeriesDifferenceStatus.items,
         }
 
         status = package_type_dsd_status[self.specified_package_type]

=== modified file 'lib/lp/registry/browser/tests/test_distroseries.py'
--- lib/lp/registry/browser/tests/test_distroseries.py	2011-07-25 11:56:30 +0000
+++ lib/lp/registry/browser/tests/test_distroseries.py	2011-07-28 13:47:52 +0000
@@ -50,7 +50,7 @@
 from lp.archivepublisher.debversion import Version
 from lp.registry.browser.distroseries import (
     HIGHER_VERSION_THAN_PARENT,
-    IGNORED,
+    ALL,
     NON_IGNORED,
     RESOLVED,
     seriesToVocab,
@@ -1611,27 +1611,23 @@
         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.
+    def test_batch_all_packages(self):
+        # field.package_type parameter allows to list all the
+        # differences.
         set_derived_series_ui_feature_flag(self)
         derived_series, parent_series = self._createChildAndParent()
-        blacklisted_diff = self.factory.makeDistroSeriesDifference(
-            derived_series=derived_series,
-            status=DistroSeriesDifferenceStatus.BLACKLISTED_CURRENT)
-
-        blacklisted_view = create_initialized_view(
+        # Create differences of all possible statuses.
+        diffs = []
+        for status in DistroSeriesDifferenceStatus.items:
+            diff = self.factory.makeDistroSeriesDifference(
+                derived_series=derived_series, status=status)
+            diffs.append(diff)
+        all_view = create_initialized_view(
             derived_series,
             '+localpackagediffs',
-            query_string='field.package_type=%s' % IGNORED)
-        unblacklisted_view = create_initialized_view(
-            derived_series,
-            '+localpackagediffs')
+            query_string='field.package_type=%s' % ALL)
 
-        self.assertContentEqual(
-            [blacklisted_diff], blacklisted_view.cached_differences.batch)
-        self.assertContentEqual(
-            [], unblacklisted_view.cached_differences.batch)
+        self.assertContentEqual(diffs, all_view.cached_differences.batch)
 
     def test_batch_blacklisted_differences_with_higher_version(self):
         # field.package_type parameter allows to list only

=== modified file 'lib/lp/registry/model/distroseriesdifference.py'
--- lib/lp/registry/model/distroseriesdifference.py	2011-07-27 08:27:09 +0000
+++ lib/lp/registry/model/distroseriesdifference.py	2011-07-28 13:47:52 +0000
@@ -439,15 +439,10 @@
         return store.add(diff)
 
     @staticmethod
-    def getForDistroSeries(
-        distro_series,
-        difference_type=None,
-        name_filter=None,
-        status=None,
-        child_version_higher=False,
-        parent_series=None,
-        packagesets=None,
-        changed_by=None):
+    def getForDistroSeries(distro_series, difference_type=None,
+                           name_filter=None, status=None,
+                           child_version_higher=False, parent_series=None,
+                           packagesets=None, changed_by=None):
         """See `IDistroSeriesDifferenceSource`."""
         if difference_type is None:
             difference_type = DistroSeriesDifferenceType.DIFFERENT_VERSIONS

=== modified file 'lib/lp/registry/templates/packagesearch-macros.pt'
--- lib/lp/registry/templates/packagesearch-macros.pt	2011-03-25 03:26:29 +0000
+++ lib/lp/registry/templates/packagesearch-macros.pt	2011-07-28 13:47:52 +0000
@@ -63,7 +63,7 @@
     class="distroseries-localdiff-search-filter"
     action="" method="GET">
     <div style="float:left; margin-right:1px;">
-      <label for="field.name_filter">Show packages with names matching:</label>
+      <label for="field.name_filter">Show packages with names or packagesets matching:</label>
     </div>
     <div style="float:left;">
       <input