launchpad-reviewers team mailing list archive

[Ian Booth <ian.booth@xxxxxxxxxxxxx>]

Ian Booth has proposed merging lp:~wallyworld/launchpad/optional-extended-choice-popup-933743 into lp:launchpad with lp:~wallyworld/launchpad/choice-popup-descriptions-932424 as a prerequisite.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #933743 in Launchpad itself: "Show bug status and importance descriptions in choice-picker"
  https://bugs.launchpad.net/launchpad/+bug/933743

For more details, see:
https://code.launchpad.net/~wallyworld/launchpad/optional-extended-choice-popup-933743/+merge/93518

== Implementation ==

We want to only show the item description in the enum popup widget when asked for.
Add include_description parameter to lazrjs vocabulary_to_choice_edit_items() method.

== Tests ==

Add new test to ensure description is only there when asked for.

== Lint ==

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/app/browser/lazrjs.py
  lib/lp/app/widgets/tests/test_itemswidgets.py
  lib/lp/bugs/browser/bugtask.py
-- 
https://code.launchpad.net/~wallyworld/launchpad/optional-extended-choice-popup-933743/+merge/93518
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wallyworld/launchpad/optional-extended-choice-popup-933743 into lp:launchpad.

=== modified file 'lib/lp/app/browser/lazrjs.py'
--- lib/lp/app/browser/lazrjs.py	2012-02-17 02:56:19 +0000
+++ lib/lp/app/browser/lazrjs.py	2012-02-17 02:56:19 +0000
@@ -526,8 +526,9 @@
 
 
 def vocabulary_to_choice_edit_items(
-    vocab, css_class_prefix=None, disabled_items=None, as_json=False,
-    name_fn=None, value_fn=None, description_fn=None):
+    vocab, include_description=False, css_class_prefix=None,
+    disabled_items=None, as_json=False, name_fn=None, value_fn=None,
+    description_fn=None):
     """Convert an enumerable to JSON for a ChoiceEdit.
 
     :vocab: The enumeration to iterate over.
@@ -558,7 +559,7 @@
         description = ''
         feature_flag = getFeatureFlag(
             'disclosure.enhanced_choice_popup.enabled')
-        if feature_flag:
+        if include_description and feature_flag:
             description = description_fn(item)
         new_item = {
             'name': name,
@@ -660,7 +661,7 @@
     def __init__(self, context, exported_field, header,
                  content_box_id=None, enum=None,
                  edit_view="+edit", edit_url=None, edit_title='',
-                 css_class_prefix=''):
+                 css_class_prefix='', include_description=False):
         """Create a widget wrapper.
 
         :param context: The object that is being edited.
@@ -689,7 +690,9 @@
             enum = exported_field.vocabulary
         if IEnumeratedType(enum, None) is None:
             raise ValueError('%r does not provide IEnumeratedType' % enum)
-        self.items = vocabulary_to_choice_edit_items(enum, css_class_prefix)
+        self.items = vocabulary_to_choice_edit_items(
+            enum, include_description=include_description,
+            css_class_prefix=css_class_prefix)
 
     @property
     def config(self):

=== modified file 'lib/lp/app/widgets/tests/test_itemswidgets.py'
--- lib/lp/app/widgets/tests/test_itemswidgets.py	2012-02-17 02:56:19 +0000
+++ lib/lp/app/widgets/tests/test_itemswidgets.py	2012-02-17 02:56:19 +0000
@@ -262,10 +262,21 @@
                     for e in self.ChoiceEnum]
         self.assertEqual(expected, items)
 
+    def test_vocabulary_to_choice_edit_items_no_description(self):
+        # Even if feature flag is on, there are no descriptions unless wanted.
+        feature_flag = {'disclosure.enhanced_choice_popup.enabled': 'on'}
+        with FeatureFixture(feature_flag):
+            overrides = {'description': ''}
+            items = vocabulary_to_choice_edit_items(self.ChoiceEnum)
+        expected = [self._makeItemDict(e.value, overrides)
+                    for e in self.ChoiceEnum]
+        self.assertEqual(expected, items)
+
     def test_vocabulary_to_choice_edit_items_with_description(self):
         # The items list is as expected with the feature flag.
         feature_flag = {'disclosure.enhanced_choice_popup.enabled': 'on'}
         with FeatureFixture(feature_flag):
-            items = vocabulary_to_choice_edit_items(self.ChoiceEnum)
+            items = vocabulary_to_choice_edit_items(
+                self.ChoiceEnum, include_description=True)
         expected = [self._makeItemDict(e.value) for e in self.ChoiceEnum]
         self.assertEqual(expected, items)

=== modified file 'lib/lp/bugs/browser/bugtask.py'
--- lib/lp/bugs/browser/bugtask.py	2012-02-08 10:43:29 +0000
+++ lib/lp/bugs/browser/bugtask.py	2012-02-17 02:56:19 +0000
@@ -46,10 +46,6 @@
     timedelta,
     )
 from itertools import groupby
-from math import (
-    floor,
-    log,
-    )
 from operator import attrgetter
 import os.path
 import re
@@ -4015,6 +4011,7 @@
 
             items = vocabulary_to_choice_edit_items(
                 SimpleVocabulary.fromItems(status_items),
+                include_description=True,
                 css_class_prefix='status',
                 disabled_items=disabled_items)
         else:
@@ -4037,6 +4034,7 @@
 
             items = vocabulary_to_choice_edit_items(
                 SimpleVocabulary.fromItems(importance_items),
+                include_description=True,
                 css_class_prefix='importance')
         else:
             items = '[]'