launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #10897
[Merge] lp:~stevenk/launchpad/drop-enhanced_choice_popup into lp:launchpad
Steve Kowalik has proposed merging lp:~stevenk/launchpad/drop-enhanced_choice_popup into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~stevenk/launchpad/drop-enhanced_choice_popup/+merge/119461
The feature flag disclosure.enhanced_choice_popup.enabled has been turned on by default for a while. Destroy it, and while I'm at it, rip out the doctest for EnumChoiceWidget and move it to the unit test.
--
https://code.launchpad.net/~stevenk/launchpad/drop-enhanced_choice_popup/+merge/119461
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stevenk/launchpad/drop-enhanced_choice_popup into lp:launchpad.
=== modified file 'lib/lp/app/browser/lazrjs.py'
--- lib/lp/app/browser/lazrjs.py 2012-08-10 04:48:36 +0000
+++ lib/lp/app/browser/lazrjs.py 2012-08-14 02:23:23 +0000
@@ -580,9 +580,7 @@
if description_fn is None:
description_fn = lambda item: getattr(item, 'description', '')
description = ''
- feature_flag = getFeatureFlag(
- 'disclosure.enhanced_choice_popup.enabled')
- if include_description and feature_flag:
+ if include_description:
description = description_fn(item)
new_item = {
'name': name,
=== modified file 'lib/lp/app/doc/lazr-js-widgets.txt'
--- lib/lp/app/doc/lazr-js-widgets.txt 2012-08-10 04:48:36 +0000
+++ lib/lp/app/doc/lazr-js-widgets.txt 2012-08-14 02:23:23 +0000
@@ -356,65 +356,6 @@
TextLineEditorWidget above.
-EnumChoiceWidget
-----------------
-
-This widget is primarily there allow setting of enum fields easily
-using the popup chooser.
-
- >>> from lp.app.browser.lazrjs import EnumChoiceWidget
-
-As with the other widgets, this one requires a context object and a Choice
-type field. The rendering of the widget hooks up to the lazr ChoiceSource
-with the standard patch plugin.
-
-One of the different things about this widget is the styles that are added.
-Many enums have specific colour styles. Generally these are the names of
-the enum values (the upper-case bit) with a defined prefix. This prefix
-is passed in to the constructor of the widget.
-
-A list of items is generated from the vocabulary.
-
-If the user does not have edit rights, the widget just renders the text based
-on the current value of the field on the object:
-
- >>> login(ANONYMOUS)
- >>> from lp.code.interfaces.branch import IBranch
- >>> branch = factory.makeAnyBranch()
- >>> widget = EnumChoiceWidget(
- ... branch, IBranch['lifecycle_status'],
- ... header='Change status to', css_class_prefix='branchstatus')
- >>> print widget()
- <span id="edit-lifecycle_status">
- <span class="value branchstatusDEVELOPMENT">Development</span>
- </span>
-
-If the user has edit rights, an edit icon is rendered and some javascript is
-rendered to hook up the widget.
-
- >>> ignored = login_person(branch.owner)
- >>> print widget()
- <span id="edit-lifecycle_status">
- <span class="value branchstatusDEVELOPMENT">Development</span>
- <span>
- <a class="editicon sprite edit action-icon"
- href="http://.../+edit";
- title="">Edit</a>
- </span>
- </span>
- <script>
- LPJS.use('lp.app.choice', function(Y) {
- ...
- </script>
-
-
-Changing the edit link
-**********************
-
-The edit link can be changed in exactly the same way as for the
-TextLineEditorWidget above.
-
-
InlineMultiCheckboxWidget
-------------------
=== modified file 'lib/lp/app/widgets/tests/test_itemswidgets.py'
--- lib/lp/app/widgets/tests/test_itemswidgets.py 2012-07-26 07:54:40 +0000
+++ lib/lp/app/widgets/tests/test_itemswidgets.py 2012-08-14 02:23:23 +0000
@@ -3,9 +3,7 @@
__metaclass__ = type
-
import doctest
-from unittest import TestCase
from lazr.enum import (
EnumeratedType,
@@ -22,17 +20,24 @@
SimpleVocabulary,
)
-from lp.app.browser.lazrjs import vocabulary_to_choice_edit_items
+from lp.app.browser.lazrjs import (
+ EnumChoiceWidget,
+ vocabulary_to_choice_edit_items,
+ )
from lp.app.widgets.itemswidgets import (
LabeledMultiCheckBoxWidget,
LaunchpadRadioWidget,
LaunchpadRadioWidgetWithDescription,
PlainMultiCheckBoxWidget,
)
-from lp.services.features.testing import FeatureFixture
+from lp.code.interfaces.branch import IBranch
from lp.services.webapp.menu import structured
from lp.services.webapp.servers import LaunchpadTestRequest
-from lp.testing import TestCaseWithFactory
+from lp.testing import (
+ ANONYMOUS,
+ person_logged_in,
+ TestCaseWithFactory,
+ )
from lp.testing.layers import DatabaseFunctionalLayer
@@ -222,13 +227,8 @@
self.assertEqual(expected, hint_html)
-class TestVocabularyToChoiceEditItems(TestCase):
- """Tests for vocabulary_to_choice_edit_items.
-
- This function is tested implicitly in lazr-js-widgets.txt.
- Here we are adding some explicit tests for the behaviour enabled by
- feature flag disclosure.enhanced_choice_popup.enabled.
- """
+class TestVocabularyToChoiceEditItems(TestCaseWithFactory):
+ """Tests for vocabulary_to_choice_edit_items and EnumChoiceWidget."""
layer = DatabaseFunctionalLayer
@@ -267,21 +267,17 @@
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)
+ # There are no descriptions unless wanted.
+ 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, include_description=True)
+ # The items list is as expected.
+ 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)
@@ -290,6 +286,50 @@
items = vocabulary_to_choice_edit_items(
self.ChoiceEnum, include_description=True,
excluded_items=[self.ChoiceEnum.ITEM_B])
- overrides = {'description': ''}
- expected = [self._makeItemDict(self.ChoiceEnum.ITEM_A, overrides)]
+ expected = [self._makeItemDict(self.ChoiceEnum.ITEM_A)]
self.assertEqual(expected, items)
+
+ def test_widget_with_anonymous_user(self):
+ # When the user is anonymous, the widget outputs static text.
+ branch = self.factory.makeAnyBranch()
+ widget = EnumChoiceWidget(
+ branch, IBranch['lifecycle_status'],
+ header='Change status to', css_class_prefix='branchstatus')
+ with person_logged_in(ANONYMOUS):
+ markup = widget()
+ expected = """
+ <span id="edit-lifecycle_status">
+ <span class="value branchstatusDEVELOPMENT">Development</span>
+ </span>
+ """
+ expected_matcher = DocTestMatches(
+ expected, (doctest.NORMALIZE_WHITESPACE |
+ doctest.REPORT_NDIFF | doctest.ELLIPSIS))
+ self.assertThat(markup, expected_matcher)
+
+ def test_widget_with_user(self):
+ # When the user has edit permission, the widget allows changes via JS.
+ branch = self.factory.makeAnyBranch()
+ widget = EnumChoiceWidget(
+ branch, IBranch['lifecycle_status'],
+ header='Change status to', css_class_prefix='branchstatus')
+ with person_logged_in(branch.owner):
+ markup = widget()
+ expected = """
+ <span id="edit-lifecycle_status">
+ <span class="value branchstatusDEVELOPMENT">Development</span>
+ <span>
+ <a class="editicon sprite edit action-icon"
+ href="http://.../+edit";
+ title="">Edit</a>
+ </span>
+ </span>
+ <script>
+ LPJS.use('lp.app.choice', function(Y) {
+ ...
+ </script>
+ """
+ expected_matcher = DocTestMatches(
+ expected, (doctest.NORMALIZE_WHITESPACE |
+ doctest.REPORT_NDIFF | doctest.ELLIPSIS))
+ self.assertThat(markup, expected_matcher)
=== modified file 'lib/lp/bugs/browser/tests/test_bugtarget_filebug.py'
--- lib/lp/bugs/browser/tests/test_bugtarget_filebug.py 2012-08-03 00:30:51 +0000
+++ lib/lp/bugs/browser/tests/test_bugtarget_filebug.py 2012-08-14 02:23:23 +0000
@@ -29,7 +29,6 @@
PRIVATE_INFORMATION_TYPES,
)
from lp.registry.interfaces.projectgroup import IProjectGroup
-from lp.services.features.testing import FeatureFixture
from lp.services.webapp.servers import LaunchpadTestRequest
from lp.testing import (
login,
@@ -551,12 +550,6 @@
layer = DatabaseFunctionalLayer
- def setUp(self):
- super(TestFileBugRequestCache, self).setUp()
- self.useFixture(FeatureFixture({
- 'disclosure.enhanced_choice_popup.enabled': 'true'
- }))
-
def _assert_cache_values(self, view, duplicate_search, private_bugs=False):
cache = IJSONRequestCache(view.request).objects
self.assertEqual(
=== modified file 'lib/lp/services/features/flags.py'
--- lib/lp/services/features/flags.py 2012-08-10 06:38:56 +0000
+++ lib/lp/services/features/flags.py 2012-08-14 02:23:23 +0000
@@ -233,13 +233,6 @@
'',
'',
''),
- ('disclosure.enhanced_choice_popup.enabled',
- 'boolean',
- ('If true, will include any available descriptive text with each choice '
- 'item in the selection popup.'),
- '',
- '',
- ''),
('disclosure.enhanced_sharing.enabled',
'boolean',
('If true, will allow the use of the new sharing view and apis used '
Follow ups
