launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #09336
[Merge] lp:~sinzui/launchpad/bug-tag-completions into lp:launchpad
Curtis Hovey has proposed merging lp:~sinzui/launchpad/bug-tag-completions into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #83736 in Launchpad itself: "Entering tags in advanced bug search requires the user to know the available tags in advance"
https://bugs.launchpad.net/launchpad/+bug/83736
Bug #184737 in Launchpad itself: "No tag autocomplete or official list in +filebug extra options"
https://bugs.launchpad.net/launchpad/+bug/184737
For more details, see:
https://code.launchpad.net/~sinzui/launchpad/bug-tag-completions/+merge/112580
Recent refactorings of the bug tag entry code make it possible to
reuse the autocompletion part on +filebug and advanced bug search.
--------------------------------------------------------------------
RULES
Pre-implementation: jcsackett
* Extract the autocompletion from setup_tag_entry() so that
other use cases can reuse it when the tag input field already
exists.
* Update +filebug to provide available_official_tags.
* setup autocomplete in the template.
* Update advanced search to provide available_official_tags.
* setup autocomplete in the template.
Addendum
* I tried to extend the Autocomplete widget, but I discovered that
I would also need to write a lot of duplicate css rules. The
subclass effectively renames all the presentation rules :(
* I tried a plugin, but it did not generate the markup before
the widget was initialised.
* I settled on a simple setup function.
QA
* Visit https://bugs.qastaging.launchpad.net/gdp/+bugs?advanced
* Verify the bug tags input will complete tags.
* Choose Report a bug.
* Enter "testing", Continue, expand Extra options.
* Verify the bug tags input will complete tags.
LINT
lib/lp/app/javascript/autocomplete/autocomplete.js
lib/lp/app/javascript/autocomplete/tests/test_autocomplete.js
lib/lp/bugs/browser/widgets/bug.py
lib/lp/bugs/browser/widgets/tests/test_bug.py
lib/lp/bugs/javascript/bug_tags_entry.js
lib/lp/bugs/javascript/tests/test_bug_tags_entry.html
lib/lp/bugs/javascript/tests/test_bug_tags_entry.js
lib/lp/bugs/templates/bugtask-macros-tableview.pt
TEST
./bin/test -vvc lp.bugs.browser.widgets.tests.test_bug
./bin/test -vvc --layer=YUITest
IMPLEMENTATION
I turn of the browser's autocomplete when the widget is run. The bug tag
completions were hidden under Chromium's completions. The browser must
be turned off for every input we add completions to.
lib/lp/app/javascript/autocomplete/autocomplete.js
lib/lp/app/javascript/autocomplete/tests/test_autocomplete.js
I extracted the setup the autocomplete widget to a separate function
that could be called without the editor. I removed the "on('queryChange'"
because I discovered it did nothing when I wrote a test for it...it
was probably used to debug key input when the widget was created.
lib/lp/bugs/javascript/bug_tags_entry.js
lib/lp/bugs/javascript/tests/test_bug_tags_entry.html
lib/lp/bugs/javascript/tests/test_bug_tags_entry.js
I Updated the BugTagsWidget to include the script to progressively
enhance any bug tag input to support autocomplete. This widget is used
both on advanced search and on +filebug. I moved the help link into the
widget to ensure it is always rendered and that it is rendered
consistently. I discovered that both +filebug and advanced bug had
duplicate ids in their crafted forms. A simple selector was not enough;
the selector must find the bug tag input field that is type="text".
lib/lp/bugs/browser/widgets/bug.py
lib/lp/bugs/browser/widgets/tests/test_bug.py
lib/lp/bugs/templates/bugtask-macros-tableview.pt
--
https://code.launchpad.net/~sinzui/launchpad/bug-tag-completions/+merge/112580
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~sinzui/launchpad/bug-tag-completions into lp:launchpad.
=== modified file 'lib/lp/app/javascript/autocomplete/autocomplete.js'
--- lib/lp/app/javascript/autocomplete/autocomplete.js 2012-05-18 22:56:13 +0000
+++ lib/lp/app/javascript/autocomplete/autocomplete.js 2012-06-28 14:39:21 +0000
@@ -198,6 +198,8 @@
'left': iregion.left + 'px',
'top': iregion.bottom + 'px'
});
+ // Disable the browser autocomplete so that it does not conflict.
+ input.set('autocomplete', 'off');
},
/**
=== modified file 'lib/lp/app/javascript/autocomplete/tests/test_autocomplete.js'
--- lib/lp/app/javascript/autocomplete/tests/test_autocomplete.js 2012-04-06 17:28:25 +0000
+++ lib/lp/app/javascript/autocomplete/tests/test_autocomplete.js 2012-06-28 14:39:21 +0000
@@ -58,6 +58,7 @@
Assert.isFalse(
autocomp.get('visible'),
"The widget should start out hidden.");
+ Assert.areEqual('off', Y.one(this.input).get('autocomplete'));
}
}));
@@ -329,13 +330,13 @@
/* A helper function to determine if two match result items are equal */
matches_are_equal: function(a, b) {
- if (typeof a == 'undefined') {
+ if (Y.Lang.isUndefined(a)) {
Assert.fail("Match set 'a' is of type 'undefined'!");
}
- if (typeof b == 'undefined') {
+ if (Y.Lang.isUndefined(b)) {
Assert.fail("Match set 'b' is of type 'undefined'!");
}
- return (a.text == b.text) && (a.offset == b.offset);
+ return (a.text === b.text) && (a.offset === b.offset);
},
test_no_matches_returns_an_empty_array: function() {
=== modified file 'lib/lp/bugs/browser/widgets/bug.py'
--- lib/lp/bugs/browser/widgets/bug.py 2011-02-24 15:30:54 +0000
+++ lib/lp/bugs/browser/widgets/bug.py 2012-06-28 14:39:21 +0000
@@ -10,7 +10,7 @@
]
import re
-
+from simplejson import dumps
from zope.app.form.browser.textwidgets import (
IntWidget,
TextAreaWidget,
@@ -26,6 +26,8 @@
from lp.app.errors import NotFoundError
from lp.app.validators import LaunchpadValidationError
from lp.bugs.interfaces.bug import IBugSet
+from lp.registry.interfaces.distribution import IDistribution
+from lp.registry.interfaces.product import IProduct
class BugWidget(IntWidget):
@@ -135,6 +137,37 @@
def _getInputValue(self):
return TextWidget.getInputValue(self)
+ def __call__(self):
+ """Return the input with a script."""
+ input_markup = super(BugTagsWidget, self).__call__()
+ script_markup = """
+ <a href="/+help-bugs/tag-search.html"
+ class="sprite maybe action-icon"
+ target="help">Tag search help</a>
+ <script type="text/javascript">
+ LPJS.use('event', 'lp.bugs.bug_tags_entry', function(Y) {
+ %s
+ Y.on('domready', function(e) {
+ Y.lp.bugs.bug_tags_entry.setup_bug_tag_complete(
+ 'input[id="field.%s"][type="text"]', official_tags);
+ });
+ });
+ </script>
+ """ % (self.official_tags_js, self.context.__name__)
+ return input_markup + script_markup
+
+ @property
+ def official_tags_js(self):
+ """Return the JavaScript of bug tags used by the bug tag completer."""
+ bug_target = self.context.context
+ pillar_target = (
+ IProduct(bug_target, None) or IDistribution(bug_target, None))
+ if pillar_target is not None:
+ official_tags = list(pillar_target.official_bug_tags)
+ else:
+ official_tags = []
+ return 'var official_tags = %s;' % dumps(official_tags)
+
class BugTagsFrozenSetWidget(BugTagsWidget):
"""A widget for editing bug tags in a `FrozenSet` field."""
=== added file 'lib/lp/bugs/browser/widgets/tests/test_bug.py'
--- lib/lp/bugs/browser/widgets/tests/test_bug.py 1970-01-01 00:00:00 +0000
+++ lib/lp/bugs/browser/widgets/tests/test_bug.py 2012-06-28 14:39:21 +0000
@@ -0,0 +1,81 @@
+# Copyright 2012 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+__metaclass__ = type
+
+from lp.bugs.browser.widgets.bug import BugTagsWidget
+from lp.bugs.interfaces.bugtarget import IHasOfficialBugTags
+from lp.services.webapp.servers import LaunchpadTestRequest
+from lp.testing import (
+ person_logged_in,
+ TestCaseWithFactory,
+ )
+from lp.testing.layers import DatabaseFunctionalLayer
+
+
+class BugTagsWidgetTestCase(TestCaseWithFactory):
+
+ layer = DatabaseFunctionalLayer
+
+ def get_widget(self, bug_target):
+ field = IHasOfficialBugTags['official_bug_tags']
+ bound_field = field.bind(bug_target)
+ request = LaunchpadTestRequest()
+ return BugTagsWidget(bound_field, None, request)
+
+ def test_official_tags_js_not_adaptable_to_product_or_distro(self):
+ # project groups are not full bug targets so they have no tags.
+ project_group = self.factory.makeProject()
+ widget = self.get_widget(project_group)
+ js = widget.official_tags_js
+ self.assertEqual('var official_tags = [];', js)
+
+ def test_official_tags_js_product_without_tags(self):
+ # Products without tags have an empty list.
+ product = self.factory.makeProduct()
+ widget = self.get_widget(product)
+ js = widget.official_tags_js
+ self.assertEqual('var official_tags = [];', js)
+
+ def test_official_tags_js_product_with_tags(self):
+ # Products with tags have a list of tags.
+ product = self.factory.makeProduct()
+ with person_logged_in(product.owner):
+ product.official_bug_tags = [u'cows', u'pigs', u'sheep']
+ widget = self.get_widget(product)
+ js = widget.official_tags_js
+ self.assertEqual('var official_tags = ["cows", "pigs", "sheep"];', js)
+
+ def test_official_tags_js_distribution_without_tags(self):
+ # Distributions without tags have an empty list.
+ distribution = self.factory.makeDistribution()
+ widget = self.get_widget(distribution)
+ js = widget.official_tags_js
+ self.assertEqual('var official_tags = [];', js)
+
+ def test_official_tags_js_distribution_with_tags(self):
+ # Distributions with tags have a list of tags.
+ distribution = self.factory.makeDistribution()
+ with person_logged_in(distribution.owner):
+ distribution.official_bug_tags = [u'cows', u'pigs', u'sheep']
+ widget = self.get_widget(distribution)
+ js = widget.official_tags_js
+ self.assertEqual('var official_tags = ["cows", "pigs", "sheep"];', js)
+
+ def test_call(self):
+ # __call__ renders the input, help link, and script with official tags.
+ # Products with tags have a list of tags.
+ product = self.factory.makeProduct()
+ with person_logged_in(product.owner):
+ product.official_bug_tags = [u'cows', u'pigs', u'sheep']
+ widget = self.get_widget(product)
+ markup = widget()
+ self.assertIn(
+ '<input class="textType" id="field.official_bug_tags"', markup)
+ self.assertIn('<a href="/+help-bugs/tag-search.html"', markup)
+ self.assertIn('var official_tags = ["cows", "pigs", "sheep"];', markup)
+ self.assertIn(
+ 'Y.lp.bugs.bug_tags_entry.setup_bug_tag_complete(', markup)
+ self.assertIn(
+ """'input[id="field.official_bug_tags"][type="text"]',""", markup)
+ self.assertIn("official_tags)", markup)
=== modified file 'lib/lp/bugs/javascript/bug_tags_entry.js'
--- lib/lp/bugs/javascript/bug_tags_entry.js 2012-06-22 19:49:29 +0000
+++ lib/lp/bugs/javascript/bug_tags_entry.js 2012-06-28 14:39:21 +0000
@@ -251,21 +251,30 @@
});
tags_trigger.addClass('js-action');
- bug_tags_div.append(
+ autocomplete = namespace.setup_bug_tag_complete(
+ '#tag-input', available_official_tags);
+};
+
+
+/**
+ * Set up bug tag autocompletion on a text input.
+ *
+ * @method setup_bug_tag_complete
+ */
+namespace.setup_bug_tag_complete = function(input, official_tags) {
+ Y.one('body').appendChild(
'<div id="tags-autocomplete"><div id="tags-autocomplete-content">' +
'</div></div>');
- autocomplete = new Y.lazr.AutoComplete({
- input: '#tag-input',
- data: available_official_tags,
+ var autocomplete = new Y.lazr.AutoComplete({
+ input: input,
+ data: official_tags,
boundingBox: '#tags-autocomplete',
contentBox: '#tags-autocomplete-content'
- });
- autocomplete.on('queryChange', function(e) {
- var val = "null";
- if (e.newVal !== null) {
- val = "'" + e.newVal.text + "', " + e.newVal.offset;
- }
- });
+ });
+ autocomplete.get('input').on('focus', function(e) {
+ autocomplete.render();
+ });
+ return autocomplete;
};
}, "0.1", {
"requires": [
=== modified file 'lib/lp/bugs/javascript/tests/test_bug_tags_entry.html'
--- lib/lp/bugs/javascript/tests/test_bug_tags_entry.html 2012-06-22 14:58:26 +0000
+++ lib/lp/bugs/javascript/tests/test_bug_tags_entry.html 2012-06-28 14:39:21 +0000
@@ -82,5 +82,14 @@
class="sprite maybe action-icon">Tag help</a>
</div>
</script>
+
+ <script type="text/x-template" id="form-with-bug-tags">
+ <form>
+ <div>
+ <input type="text" name="field.tag" id="field.tag" />
+ <a href="/help/tags.html" target="help" class="help">(?)</a>
+ </div>
+ </form>
+ </script>
</body>
</html>
=== modified file 'lib/lp/bugs/javascript/tests/test_bug_tags_entry.js'
--- lib/lp/bugs/javascript/tests/test_bug_tags_entry.js 2012-06-22 19:49:29 +0000
+++ lib/lp/bugs/javascript/tests/test_bug_tags_entry.js 2012-06-28 14:39:21 +0000
@@ -87,12 +87,7 @@
Y.Assert.isInstanceOf(Y.Node, form_node.one('#tags-edit-spinner'));
Y.Assert.isInstanceOf(Y.Node, form_node.one('#edit-tags-cancel'));
Y.Assert.isInstanceOf(Y.Node, form_node.one('#tag-input'));
- // The Autocompleter nodes are provided.
- var completer_node = this.bug_tags_div.one('#tags-autocomplete');
- Y.Assert.isInstanceOf(Y.Node, completer_node);
- var completer_content = completer_node.one(
- '#tags-autocomplete-content');
- Y.Assert.isInstanceOf(Y.Node, completer_content);
+ Y.Assert.isInstanceOf(Y.Node, Y.one('#tags-autocomplete'));
},
test_show_activity: function() {
@@ -206,5 +201,47 @@
}
}));
+ suite.add(new Y.Test.Case({
+ name: 'Completer',
+
+ setUp: function() {
+ this.fixture = Y.one("#fixture");
+ var template = Y.one('#form-with-bug-tags').getContent();
+ this.fixture.append(template);
+ },
+
+ tearDown: function() {
+ if (this.fixture !== null) {
+ this.fixture.empty();
+ }
+ delete this.fixture;
+ },
+
+ test_setup_bug_tag_complete: function() {
+ // The Autocompleter nodes are provided.
+ module.setup_bug_tag_complete(
+ 'input[id=field.tag]',['project-tag']);
+ var completer_node = Y.one('#tags-autocomplete');
+ Y.Assert.isInstanceOf(Y.Node, completer_node);
+ var completer_content = completer_node.one(
+ '#tags-autocomplete-content');
+ Y.Assert.isInstanceOf(Y.Node, completer_content);
+ var input = Y.one('input[id=field.tag]');
+ },
+
+ test_render_on_focus: function() {
+ // The Autocompleter nodes are provided.
+ var completer = module.setup_bug_tag_complete(
+ 'input[id=field.tag]',['project-tag']);
+ var input = Y.one('input[id=field.tag]');
+ var called = false;
+ completer.render = function() {
+ called = true;
+ };
+ input.simulate('focus');
+ Y.Assert.isTrue(called);
+ }
+ }));
+
Y.lp.testing.Runner.run(suite);
});
=== modified file 'lib/lp/bugs/templates/bugtask-macros-tableview.pt'
--- lib/lp/bugs/templates/bugtask-macros-tableview.pt 2012-03-01 19:12:34 +0000
+++ lib/lp/bugs/templates/bugtask-macros-tableview.pt 2012-06-28 14:39:21 +0000
@@ -516,8 +516,6 @@
tal:content="structure view/widgets/tag/label">
Tag
</label>: <input tal:replace="structure view/widgets/tag" />
- <a href="/+help-bugs/tag-search.html"
- target="help">Tag search help</a>
<div
tal:condition="error"
class="message"
Follow ups
