launchpad-reviewers team mailing list archive

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

Raphaël Victor Badin has proposed merging lp:~rvb/launchpad/db-dds-fix-filter-form-add-parent-name into lp:launchpad/db-devel.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #739517 in Launchpad itself: "Source package difference form should mention the parent's name."
  https://bugs.launchpad.net/launchpad/+bug/739517

For more details, see:
https://code.launchpad.net/~rvb/launchpad/db-dds-fix-filter-form-add-parent-name/+merge/54239

This branch fixes the display of the radio button used on the source package difference page.  It uses the actual name of the parent series instead of just 'the parent'.

It targets db-devel because it's a simple fix over a branch which targeted db-devel.
-- 
https://code.launchpad.net/~rvb/launchpad/db-dds-fix-filter-form-add-parent-name/+merge/54239
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/launchpad/db-dds-fix-filter-form-add-parent-name into lp:launchpad/db-devel.

=== modified file 'lib/lp/registry/browser/distroseries.py'
--- lib/lp/registry/browser/distroseries.py	2011-03-18 15:39:35 +0000
+++ lib/lp/registry/browser/distroseries.py	2011-03-21 17:08:34 +0000
@@ -535,13 +535,18 @@
 
 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')))
+
+def make_package_type_vocabulary(parent_name):
+    return 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 '%s'"
+                % parent_name)))
 
 
 class DistroSeriesNeedsPackagesView(LaunchpadView):
@@ -563,11 +568,6 @@
     name_filter = TextLine(
         title=_("Package name contains"), required=False)
 
-    package_type = Choice(
-        vocabulary=PACKAGE_TYPE_VOCABULARY,
-        default=DEFAULT_PACKAGE_TYPE,
-        required=True)
-
     selected_differences = List(
         title=_('Selected differences'),
         value_type=Choice(vocabulary=SimpleVocabulary([])),
@@ -578,7 +578,7 @@
 class DistroSeriesLocalDifferences(LaunchpadFormView):
     """Present differences between a derived series and its parent."""
     schema = IDifferencesFormSchema
-    field_names = ['selected_differences', 'package_type']
+    field_names = ['selected_differences']
     custom_widget('selected_differences', LabeledMultiCheckBoxWidget)
     custom_widget('package_type', LaunchpadRadioWidget)
 
@@ -608,6 +608,14 @@
                 self.context.parent_series.displayname,
                 ))
 
+    def setupPackageFilterRadio(self):
+        return form.Fields(Choice(
+            __name__='package_type',
+            vocabulary=make_package_type_vocabulary(
+                self.context.parent_series.displayname),
+            default=DEFAULT_PACKAGE_TYPE,
+            required=True))
+
     def setUpFields(self):
         """Add the selected differences field.
 
@@ -615,6 +623,10 @@
         for its own vocabulary, we set it up after all the others.
         """
         super(DistroSeriesLocalDifferences, self).setUpFields()
+        self.form_fields = (
+            self.setupPackageFilterRadio() +
+            self.form_fields)
+
         has_edit = check_permission('launchpad.Edit', self.context)
 
         terms = [

=== modified file 'lib/lp/registry/browser/tests/test_series_views.py'
--- lib/lp/registry/browser/tests/test_series_views.py	2011-03-21 15:09:25 +0000
+++ lib/lp/registry/browser/tests/test_series_views.py	2011-03-21 17:08:34 +0000
@@ -3,13 +3,14 @@
 
 __metaclass__ = type
 
+import unittest
+
 from BeautifulSoup import BeautifulSoup
+import soupmatchers
 from storm.zope.interfaces import IResultSet
 from zope.component import getUtility
 from zope.security.proxy import removeSecurityProxy
 
-import unittest
-
 from canonical.config import config
 from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
 from canonical.launchpad.testing.pages import find_tag_by_id
@@ -17,31 +18,32 @@
 from canonical.launchpad.webapp.publisher import canonical_url
 from canonical.testing.layers import (
     DatabaseFunctionalLayer,
+    LaunchpadFunctionalLayer,
     LaunchpadZopelessLayer,
-    LaunchpadFunctionalLayer,
+    )
+from lp.registry.browser.distroseries import (
+    BLACKLISTED,
+    HIGHER_VERSION_THAN_PARENT,
+    NON_BLACKLISTED,
     )
 from lp.registry.enum import (
     DistroSeriesDifferenceStatus,
     DistroSeriesDifferenceType,
     )
+from lp.services.features import (
+    getFeatureFlag,
+    install_feature_controller,
+    per_thread,
+    )
 from lp.services.features.flags import FeatureController
 from lp.services.features.model import (
     FeatureFlag,
     getFeatureStore,
     )
-from lp.services.features import (
-    getFeatureFlag,
-    install_feature_controller,
-    )
 from lp.testing import (
-    TestCaseWithFactory,
     login_person,
     person_logged_in,
-    )
-from lp.registry.browser.distroseries import (
-    BLACKLISTED,
-    NON_BLACKLISTED,
-    HIGHER_VERSION_THAN_PARENT,
+    TestCaseWithFactory,
     )
 from lp.testing.views import create_initialized_view
 
@@ -272,6 +274,28 @@
 
     layer = LaunchpadFunctionalLayer
 
+    def test_higher_radio_mentions_parent(self):
+        set_derived_series_ui_feature_flag(self)
+        derived_series = self.factory.makeDistroSeries(
+            name='derilucid', parent_series=self.factory.makeDistroSeries(
+                name='lucid',
+                displayname='Lucid'))
+        diff1 = self.factory.makeDistroSeriesDifference(
+            derived_series=derived_series,
+            source_package_name_str="my-src-package")
+        view = create_initialized_view(
+            derived_series,
+            '+localpackagediffs')
+
+        radio_title = \
+            " Blacklisted packages with a higher version than in 'Lucid'"
+        radio_option_matches = soupmatchers.HTMLContains(
+            soupmatchers.Tag(
+                "radio displays parent's name", 'label',
+                text=radio_title),
+            )
+        self.assertThat(view.render(), radio_option_matches)
+
     def test_batch_filtered(self):
         # The name_filter parameter allows to filter packages by name.
         set_derived_series_ui_feature_flag(self)
@@ -298,7 +322,7 @@
         self.assertContentEqual(
             [diff2, diff1], unfiltered_view.cached_differences.batch)
 
-    def test_batch_unfiltered(self):
+    def test_batch_non_blacklisted(self):
         # The default filter is all non blacklisted differences.
         set_derived_series_ui_feature_flag(self)
         derived_series = self.factory.makeDistroSeries(
@@ -317,7 +341,7 @@
         filtered_view = create_initialized_view(
             derived_series,
             '+localpackagediffs',
-            query_string='field.package_type=%s' %NON_BLACKLISTED)
+            query_string='field.package_type=%s' % NON_BLACKLISTED)
         filtered_view2 = create_initialized_view(
             derived_series,
             '+localpackagediffs')
@@ -341,7 +365,7 @@
         blacklisted_view = create_initialized_view(
             derived_series,
             '+localpackagediffs',
-            query_string='field.package_type=%s' %BLACKLISTED)
+            query_string='field.package_type=%s' % BLACKLISTED)
         unblacklisted_view = create_initialized_view(
             derived_series,
             '+localpackagediffs')
@@ -370,7 +394,7 @@
         blacklisted_view = create_initialized_view(
             derived_series,
             '+localpackagediffs',
-            query_string='field.package_type=%s' %HIGHER_VERSION_THAN_PARENT)
+            query_string='field.package_type=%s' % HIGHER_VERSION_THAN_PARENT)
         unblacklisted_view = create_initialized_view(
             derived_series,
             '+localpackagediffs')