launchpad-reviewers team mailing list archive

[Steve Kowalik <stevenk@xxxxxxxxxx>]

Steve Kowalik has proposed merging lp:~stevenk/launchpad/hack-itemwidget into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #992384 in Launchpad itself: "Bug:+filebug and Bug:+secrecy do not respect display_userdata_as_private"
  https://bugs.launchpad.net/launchpad/+bug/992384

For more details, see:
https://code.launchpad.net/~stevenk/launchpad/hack-itemwidget/+merge/104202

Currently, if both show_information_type_in_ui and display_userdata_as_private are enabled, the two pages that make use of LaunchpadRadioWidgetWithDescription (Bug:+secrecy and Bug:+filebug) correctly show one of the radio buttons as 'Private', but the description contains 'user data', not 'private information' as it expected.

I have tracked this down to the widget itself, and the slightly funky behaviour we have due to the vocab being backed by an enum, and as such, the widget will use term.description if it is set, otherwise falls back to term.value.description.

I have done a drive-by of fixing the imports in test_itemswidgets.
-- 
https://code.launchpad.net/~stevenk/launchpad/hack-itemwidget/+merge/104202
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stevenk/launchpad/hack-itemwidget into lp:launchpad.

=== modified file 'lib/lp/app/widgets/itemswidgets.py'
--- lib/lp/app/widgets/itemswidgets.py	2012-01-17 04:44:17 +0000
+++ lib/lp/app/widgets/itemswidgets.py	2012-05-01 05:15:23 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Widgets dealing with a choice of options."""
@@ -159,7 +159,10 @@
         """Render the table row for the widget depending on description."""
         if form_value != self._missing:
             vocab_term = self.vocabulary.getTermByToken(form_value)
-            description = vocab_term.value.description
+            # This is not needed when display_userdata_as_private is removed.
+            description = getattr(vocab_term, 'description', None)
+            if description is None:
+                description = vocab_term.value.description
         else:
             description = None
 

=== modified file 'lib/lp/app/widgets/tests/test_itemswidgets.py'
--- lib/lp/app/widgets/tests/test_itemswidgets.py	2012-02-28 04:24:19 +0000
+++ lib/lp/app/widgets/tests/test_itemswidgets.py	2012-05-01 05:15:23 +0000
@@ -1,24 +1,20 @@
-# Copyright 2011 Canonical Ltd.  This software is licensed under the
+# Copyright 2011-2012 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
-from unittest import TestCase
-
-from lazr.enum._enum import (
-    DBEnumeratedType,
-    DBItem,
-    )
-
-from lp.app.browser.lazrjs import vocabulary_to_choice_edit_items
-from lp.services.features.testing import FeatureFixture
-
 
 __metaclass__ = type
 
+
 import doctest
+from unittest import TestCase
 
 from lazr.enum import (
     EnumeratedType,
     Item,
     )
+from lazr.enum._enum import (
+    DBEnumeratedType,
+    DBItem,
+    )
 from testtools.matchers import DocTestMatches
 from zope.schema import Choice
 from zope.schema.vocabulary import (
@@ -26,12 +22,15 @@
     SimpleVocabulary,
     )
 
+from lp.app.browser.lazrjs import vocabulary_to_choice_edit_items
 from lp.app.widgets.itemswidgets import (
     LabeledMultiCheckBoxWidget,
     LaunchpadRadioWidget,
     LaunchpadRadioWidgetWithDescription,
     PlainMultiCheckBoxWidget,
     )
+from lp.registry.vocabularies import InformationTypeVocabulary
+from lp.services.features.testing import FeatureFixture
 from lp.services.webapp.menu import structured
 from lp.services.webapp.servers import LaunchpadTestRequest
 from lp.testing import TestCaseWithFactory
@@ -223,6 +222,19 @@
         hint_html = self.widget.renderExtraHint()
         self.assertEqual(expected, hint_html)
 
+    def test_renderDescription(self):
+        # If the vocabulary provides a description property, it is used over
+        # the one provided by the enum.
+        feature_flag = {
+            'disclosure.display_userdata_as_private.enabled': 'on'}
+        with FeatureFixture(feature_flag):
+            vocab = InformationTypeVocabulary()
+            widget = LaunchpadRadioWidgetWithDescription(
+                self.field, vocab, self.request)
+            self.assertRenderItem(
+                "... containing private information ...", widget.renderItem,
+                vocab.getTermByToken('USERDATA'))
+
 
 class TestVocabularyToChoiceEditItems(TestCase):
     """Tests for vocabulary_to_choice_edit_items.

=== modified file 'lib/lp/registry/vocabularies.py'
--- lib/lp/registry/vocabularies.py	2012-04-26 00:13:47 +0000
+++ lib/lp/registry/vocabularies.py	2012-05-01 05:15:23 +0000
@@ -2252,6 +2252,7 @@
                 description = (
                     description.replace('user data', 'private information'))
             term = SimpleTerm(type, type.name, title)
+            term.name = type.name
             term.description = description
             terms.append(term)
         super(InformationTypeVocabulary, self).__init__(terms)