launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #08268
[Merge] lp:~wallyworld/launchpad/filebug-choice-widgets-1003748 into lp:launchpad
Ian Booth has proposed merging lp:~wallyworld/launchpad/filebug-choice-widgets-1003748 into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #1003748 in Launchpad itself: "filebug form choice widgets for importance, info type, "
https://bugs.launchpad.net/launchpad/+bug/1003748
For more details, see:
https://code.launchpad.net/~wallyworld/launchpad/filebug-choice-widgets-1003748/+merge/107616
== Implementation ==
The branch provides functionality to enhance the +filebug form when javascript is enabled. The standard form uses simple drop down combo controls for bug status and importance. These controls are replaced with choice source popups the same as used on the bugtask form itself. So the user gets nice colours and descriptions etc.
The way it works is that the drop down combo is hidden and the choice source popup is rendered in its place. The the popup choice is made, it is saved to the hidden combo widget so that when the form is saved, the correct value is passed to the back end.
I have made an addition change for the popups. Not all of the available bug status choices are valid when filing a bug. Specifically, these choices have been excluded from selection:
- expired
- invalid
- opinion
- won't fix
- incomplete
So the user gets to choose from:
- new
- confirmed
- triaged
- in progress
- fix committed
- fix released
The latter 2 choices allow a bug to be file retrospectively for a fix that has already been completed.
== Tests ==
Add new yui tests for the additional file bug functionality.
Update the TestFileBugRequestCache test case to check the additional json request cache attributes for status and importance.
Update TestVocabularyToChoiceEditItems to check that vocabulary_to_choice_edit_items() handles the excluded_items parameter.
= Lint =
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/app/browser/lazrjs.py
lib/lp/app/javascript/choice.js
lib/lp/app/widgets/tests/test_itemswidgets.py
lib/lp/bugs/browser/bugtarget.py
lib/lp/bugs/browser/tests/test_bugtarget_filebug.py
lib/lp/bugs/javascript/filebug.js
lib/lp/bugs/javascript/tests/test_filebug.html
lib/lp/bugs/javascript/tests/test_filebug.js
--
https://code.launchpad.net/~wallyworld/launchpad/filebug-choice-widgets-1003748/+merge/107616
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wallyworld/launchpad/filebug-choice-widgets-1003748 into lp:launchpad.
=== modified file 'lib/lp/app/browser/lazrjs.py'
--- lib/lp/app/browser/lazrjs.py 2012-03-05 09:17:17 +0000
+++ lib/lp/app/browser/lazrjs.py 2012-05-28 22:54:19 +0000
@@ -513,8 +513,8 @@
def vocabulary_to_choice_edit_items(
vocab, include_description=False, css_class_prefix=None,
- disabled_items=None, as_json=False, name_fn=None, value_fn=None,
- description_fn=None):
+ disabled_items=None, excluded_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.
@@ -532,6 +532,8 @@
# SimpleTerm objects have the object itself at item.value.
if safe_hasattr(item, 'value'):
item = item.value
+ if excluded_items and item in excluded_items:
+ continue
if name_fn is not None:
name = name_fn(item)
else:
=== modified file 'lib/lp/app/javascript/choice.js'
--- lib/lp/app/javascript/choice.js 2012-05-15 16:51:06 +0000
+++ lib/lp/app/javascript/choice.js 2012-05-28 22:54:19 +0000
@@ -56,4 +56,43 @@
widget.render();
};
+namespace.addPopupChoice = function(field_name, choices) {
+ var legacy_field_node = Y.one('[id=field.' + field_name + ']');
+ var initial_field_value = legacy_field_node.get('value');
+
+ var choice_node = Y.Node.create([
+ '<span id="' + field_name + '-content"><span class="value"></span>',
+ '<a href="#"><img class="sprite edit editicon"/> </a></span>'
+ ].join(' '));
+
+ legacy_field_node.insertBefore(choice_node, legacy_field_node);
+ legacy_field_node.addClass('unseen');
+ var field_content = Y.one('#' + field_name + '-content');
+
+ var choice_edit = new Y.ChoiceSource({
+ contentBox: field_content,
+ value: initial_field_value,
+ title: 'Set ' + field_name + " as",
+ items: choices,
+ elementToFlash: field_content
+ });
+ choice_edit.render();
+
+ var update_selected_value_css = function(selected_value) {
+ Y.Array.each(choices, function(item) {
+ if (item.value === selected_value) {
+ field_content.addClass(item.css_class);
+ } else {
+ field_content.removeClass(item.css_class);
+ }
+ });
+ };
+ update_selected_value_css(initial_field_value);
+ choice_edit.on('save', function(e) {
+ var selected_value = choice_edit.get('value');
+ update_selected_value_css(selected_value);
+ legacy_field_node.set('value', selected_value);
+ });
+};
+
}, "0.1", {"requires": ["lazr.choiceedit", "lp.client.plugins"]});
=== modified file 'lib/lp/app/widgets/tests/test_itemswidgets.py'
--- lib/lp/app/widgets/tests/test_itemswidgets.py 2012-05-16 23:56:08 +0000
+++ lib/lp/app/widgets/tests/test_itemswidgets.py 2012-05-28 22:54:19 +0000
@@ -298,3 +298,12 @@
self.ChoiceEnum, include_description=True)
expected = [self._makeItemDict(e.value) for e in self.ChoiceEnum]
self.assertEqual(expected, items)
+
+ def test_vocabulary_to_choice_edit_items_excluded_items(self):
+ # Excluded items are not included.
+ 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)]
+ self.assertEqual(expected, items)
=== modified file 'lib/lp/bugs/browser/bugtarget.py'
--- lib/lp/bugs/browser/bugtarget.py 2012-05-25 14:38:42 +0000
+++ lib/lp/bugs/browser/bugtarget.py 2012-05-28 22:54:19 +0000
@@ -58,6 +58,7 @@
LaunchpadFormView,
safe_action,
)
+from lp.app.browser.lazrjs import vocabulary_to_choice_edit_items
from lp.app.browser.stringformatter import FormattersAPI
from lp.app.enums import ServiceUsage
from lp.app.errors import (
@@ -98,6 +99,8 @@
IOfficialBugTagTargetRestricted,
)
from lp.bugs.interfaces.bugtask import (
+ BugTaskImportance,
+ BugTaskStatus,
IBugTaskSet,
UNRESOLVED_BUGTASK_STATUSES,
)
@@ -389,6 +392,21 @@
cache.objects['enable_bugfiling_duplicate_search'] = (
IProjectGroup.providedBy(self.context)
or self.context.enable_bugfiling_duplicate_search)
+ bugtask_status_data = vocabulary_to_choice_edit_items(
+ BugTaskStatus, include_description=True, css_class_prefix='status',
+ excluded_items=[
+ BugTaskStatus.UNKNOWN,
+ BugTaskStatus.EXPIRED,
+ BugTaskStatus.INVALID,
+ BugTaskStatus.OPINION,
+ BugTaskStatus.WONTFIX,
+ BugTaskStatus.INCOMPLETE])
+ cache.objects['bugtask_status_data'] = bugtask_status_data
+ bugtask_importance_data = vocabulary_to_choice_edit_items(
+ BugTaskImportance, include_description=True,
+ css_class_prefix='importance',
+ excluded_items=[BugTaskImportance.UNKNOWN])
+ cache.objects['bugtask_importance_data'] = bugtask_importance_data
if (self.extra_data_token is not None and
not self.extra_data_to_process):
# self.extra_data has been initialized in publishTraverse().
=== modified file 'lib/lp/bugs/browser/tests/test_bugtarget_filebug.py'
--- lib/lp/bugs/browser/tests/test_bugtarget_filebug.py 2012-05-27 23:04:28 +0000
+++ lib/lp/bugs/browser/tests/test_bugtarget_filebug.py 2012-05-28 22:54:19 +0000
@@ -21,6 +21,10 @@
IBugAddForm,
IBugSet,
)
+from lp.bugs.interfaces.bugtask import (
+ BugTaskImportance,
+ BugTaskStatus,
+ )
from lp.bugs.publisher import BugsLayer
from lp.registry.enums import (
InformationType,
@@ -496,10 +500,59 @@
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):
cache = IJSONRequestCache(view.request).objects
self.assertEqual(
- cache['enable_bugfiling_duplicate_search'], duplicate_search)
+ duplicate_search, cache['enable_bugfiling_duplicate_search'])
+ excluded_statuses = [
+ BugTaskStatus.UNKNOWN,
+ BugTaskStatus.EXPIRED,
+ BugTaskStatus.INVALID,
+ BugTaskStatus.OPINION,
+ BugTaskStatus.WONTFIX,
+ BugTaskStatus.INCOMPLETE]
+ bugtask_status_data = []
+ for item in BugTaskStatus:
+ item = item.value
+ if item in excluded_statuses:
+ continue
+ name = item.title
+ value = item.title
+ description = item.description
+ new_item = {'name': name, 'value': value,
+ 'description': description,
+ 'description_css_class': 'choice-description',
+ 'style': '',
+ 'help': '', 'disabled': False,
+ 'css_class': 'status' + item.name}
+ bugtask_status_data.append(new_item)
+ self.assertEqual(
+ bugtask_status_data, cache['bugtask_status_data'])
+ excluded_importances = [
+ BugTaskImportance.UNKNOWN]
+ bugtask_importance_data = []
+ for item in BugTaskImportance:
+ item = item.value
+ if item in excluded_importances:
+ continue
+ name = item.title
+ value = item.title
+ description = item.description
+ new_item = {'name': name, 'value': value,
+ 'description': description,
+ 'description_css_class': 'choice-description',
+ 'style': '',
+ 'help': '', 'disabled': False,
+ 'css_class': 'importance' + item.name}
+ bugtask_importance_data.append(new_item)
+ self.assertEqual(
+ bugtask_importance_data, cache['bugtask_importance_data'])
def test_product(self):
project = self.factory.makeProduct(official_malone=True)
=== modified file 'lib/lp/bugs/javascript/filebug.js'
--- lib/lp/bugs/javascript/filebug.js 2012-05-25 14:38:42 +0000
+++ lib/lp/bugs/javascript/filebug.js 2012-05-28 22:54:19 +0000
@@ -28,6 +28,7 @@
} else {
setup_security_related();
}
+ setupChoiceWidgets();
var filebug_privacy_text = "This report will be private. " +
"You can disclose it later.";
update_privacy_banner(
@@ -53,6 +54,12 @@
}, "input[name='field.information_type']");
};
+var setupChoiceWidgets = function() {
+ Y.lp.app.choice.addPopupChoice('status', LP.cache.bugtask_status_data);
+ Y.lp.app.choice.addPopupChoice(
+ 'importance', LP.cache.bugtask_importance_data);
+};
+
var setup_security_related = function() {
var sec = Y.one('[id="field.security_related"]');
if (!Y.Lang.isValue(sec)) {
@@ -71,5 +78,6 @@
namespace.setup_filebug = setup_filebug;
}, "0.1", {"requires": [
- "base", "node", "event", "node-event-delegate",
- "lp.app.banner.privacy", "lp.bugs.filebug_dupefinder"]});
+ "base", "node", "event", "node-event-delegate", "lazr.choiceedit",
+ "lp.app.banner.privacy", "lp.app.choice",
+ "lp.bugs.filebug_dupefinder"]});
=== modified file 'lib/lp/bugs/javascript/tests/test_filebug.html'
--- lib/lp/bugs/javascript/tests/test_filebug.html 2012-05-25 14:38:42 +0000
+++ lib/lp/bugs/javascript/tests/test_filebug.html 2012-05-28 22:54:19 +0000
@@ -28,6 +28,10 @@
<script type="text/javascript"
src="../../../../../build/js/lp/app/mustache.js"></script>
<script type="text/javascript"
+ src="../../../../../build/js/lp/app/choice.js"></script>
+ <script type="text/javascript"
+ src="../../../../../build/js/lp/app/choiceedit/choiceedit.js"></script>
+ <script type="text/javascript"
src="../../../../../build/js/lp/app/expander.js"></script>
<script type="text/javascript"
src="../../../../../build/js/lp/app/formoverlay/formoverlay.js"></script>
@@ -98,6 +102,18 @@
</tr>
</tbody>
</table>
+ <div class="value">
+ <select size="1" name="field.status" id="field.status">
+ <option value="New" selected="selected">New</option>
+ <option value="Incomplete">Incomplete</option>
+ </select>
+ </div>
+ <div class="value">
+ <select size="1" name="field.status" id="field.importance">
+ <option value="Undecided" selected="selected">Undecided</option>
+ <option value="High">High</option>
+ </select>
+ </div>
</script>
</body>
</html>
=== modified file 'lib/lp/bugs/javascript/tests/test_filebug.js'
--- lib/lp/bugs/javascript/tests/test_filebug.js 2012-05-25 14:38:42 +0000
+++ lib/lp/bugs/javascript/tests/test_filebug.js 2012-05-28 22:54:19 +0000
@@ -13,7 +13,15 @@
window.LP = {
links: {},
cache: {
- private_types: ['EMBARGOEDSECURITY', 'USERDATA']
+ private_types: ['EMBARGOEDSECURITY', 'USERDATA'],
+ bugtask_status_data: [
+ {name: 'New', value: 'New'},
+ {name: 'Incomplete', value: 'Incomplete'}
+ ],
+ bugtask_importance_data: [
+ {name: 'Undecided', value: 'Undecided'},
+ {name: 'High', value: 'High'}
+ ]
}
};
this.fixture = Y.one('#fixture');
@@ -104,10 +112,56 @@
Y.lp.bugs.filebug_dupefinder.setup_dupes = orig_setup_dupes;
Y.lp.bugs.filebug_dupefinder.setup_dupe_finder
= orig_setup_dupe_finder;
+ },
+
+ // The bugtask status choice popup is rendered.
+ test_status_setup: function () {
+ Y.lp.bugs.filebug.setup_filebug(true);
+ var status_node = Y.one('#status-content .value');
+ Y.Assert.areEqual('New', status_node.get('text'));
+ var status_edit_node = Y.one('#status-content a img.edit');
+ Y.Assert.isNotNull(status_edit_node);
+ var legacy_dropdown = Y.one('[id=field.status]');
+ Y.Assert.isTrue(legacy_dropdown.hasClass('unseen'));
+ },
+
+ // The bugtask importance choice popup is rendered.
+ test_importance_setup: function () {
+ Y.lp.bugs.filebug.setup_filebug(true);
+ var importance_node = Y.one('#importance-content .value');
+ Y.Assert.areEqual('Undecided', importance_node.get('text'));
+ var importance_edit_node = Y.one('#status-content a img.edit');
+ Y.Assert.isNotNull(importance_edit_node);
+ var legacy_dropdown = Y.one('[id=field.importance]');
+ Y.Assert.isTrue(legacy_dropdown.hasClass('unseen'));
+ },
+
+ // The bugtask status choice popup updates the form.
+ test_status_selection: function() {
+ Y.lp.bugs.filebug.setup_filebug(true);
+ var status_popup = Y.one('#status-content a');
+ status_popup.simulate('click');
+ var status_choice = Y.one(
+ '.yui3-ichoicelist-content a[href=#Incomplete]');
+ status_choice.simulate('click');
+ var legacy_dropdown = Y.one('[id=field.status]');
+ Y.Assert.areEqual('Incomplete', legacy_dropdown.get('value'));
+ },
+
+ // The bugtask importance choice popup updates the form.
+ test_importance_selection: function() {
+ Y.lp.bugs.filebug.setup_filebug(true);
+ var status_popup = Y.one('#importance-content a');
+ status_popup.simulate('click');
+ var status_choice = Y.one(
+ '.yui3-ichoicelist-content a[href=#High]');
+ status_choice.simulate('click');
+ var legacy_dropdown = Y.one('[id=field.importance]');
+ Y.Assert.areEqual('High', legacy_dropdown.get('value'));
}
}));
}, '0.1', {'requires': ['test', 'console', 'event', 'node-event-simulate',
- 'lp.app.banner.privacy', 'lp.bugs.filebug_dupefinder',
- 'lp.bugs.filebug'
+ 'lp.app.banner.privacy', 'lp.app.choice',
+ 'lp.bugs.filebug_dupefinder', 'lp.bugs.filebug'
]});
Follow ups
