launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #10143
[Merge] lp:~sinzui/launchpad/adapt-subscription-to-pillar into lp:launchpad
Curtis Hovey has proposed merging lp:~sinzui/launchpad/adapt-subscription-to-pillar into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #780568 in Launchpad itself: "bug structural subscription auto tag complete"
https://bugs.launchpad.net/launchpad/+bug/780568
For more details, see:
https://code.launchpad.net/~sinzui/launchpad/adapt-subscription-to-pillar/+merge/116095
bug tags do not complete because the IBugSubscriptionFilter
class cannot be adapted to an IProduct or IDistribution to provide
the official list of bug tags. Adding autocomplete to the filter's
input field is two lines of code.
--------------------------------------------------------------------
RULES
Pre-implementation: no one
* Create an adapter for IProduct and IDistribution
* Verify autocompletion works in the HTML forms
* Call bug_tags_entry() when the input is created and pass in the
official list of bug tags.
ADDENDUM
* OMG! This was so must harder than is should be.
* We have never used autocomplete in an overlay before...It runs but
you cannot see it because it is 1000 z-index's below the overlay!
* The bug tag completer setup code can only be called once because
it uses ids!
* The structural subscription module is just a namespace of procedural
code with few entry points to call, return, or instrument to test!
QA
* https://bugs.qastaging.launchpad.net/~/+structural-subscriptions
* Choose Create new filter
* Verify that tags autocomplete
* https://bugs.qastaging.launchpad.net/launchpad
* Choose Subscribe to bug mail and expand the tag section
* Verify the tag editor completes.
* Save the subscription
* Choose Edit bug mail
* Edit a subscription, expand the tags
* Enter text to see the suggestions, press Esc
* Edit a different subscription, expand the tags
* Enter text, verify you do not see the previous list of suggests...
you only see the list for what you are typing.
LINT
lib/lp/app/javascript/autocomplete/autocomplete.js
lib/lp/app/javascript/autocomplete/tests/test_autocomplete.js
lib/lp/bugs/configure.zcml
lib/lp/bugs/adapters/bugsubscriptionfilter.py
lib/lp/bugs/adapters/tests/test_bugsubscriptionfilter.py
lib/lp/bugs/javascript/bug_tags_entry.js
lib/lp/bugs/javascript/tests/test_bug_tags_entry.js
lib/lp/registry/javascript/structural-subscription.js
lib/lp/registry/javascript/tests/test_structural_subscription.html
lib/lp/registry/javascript/tests/test_structural_subscription.js
TEST
./bin/test -vv lp.bugs.adapters.tests.test_bugsubscriptionfilter
./bin/test -vvc --layer=YUITest lp
IMPLEMENTATION
Let me see the autocomplete list that decorates the input field. This is
set at 31000 because someone thing might need to be higher, but in general
user input is always the highest thing on the stack, so autocomplete should
be the highest.
lib/lp/app/javascript/autocomplete/autocomplete.js
lib/lp/app/javascript/autocomplete/tests/test_autocomplete.js
Created and registered adapters that convert the bugsubscriptionfilter to
their product or distribution. The bug tag completer code knows how to
generate the list of official tags when the pillar is available.
lib/lp/bugs/configure.zcml
lib/lp/bugs/adapters/bugsubscriptionfilter.py
lib/lp/bugs/adapters/tests/test_bugsubscriptionfilter.py
I discovered that the +subscriptions view created multiple overlays, each
with its own autocomplete. Editing failed after a few uses because the
bug_tags_entry() assumes it is used once and can use an id.
lib/lp/bugs/javascript/bug_tags_entry.js
lib/lp/bugs/javascript/tests/test_bug_tags_entry.js
I added setup_tag_complete() with the input and official bug tags. The
completer has to be cleaned up to ensure there is only one list of
suggestions in existence at any one time. I made the tag split() code
more robust by stripping leading and trailing white-space.
lib/lp/registry/javascript/structural-subscription.js
lib/lp/registry/javascript/tests/test_structural_subscription.html
lib/lp/registry/javascript/tests/test_structural_subscription.js
--
https://code.launchpad.net/~sinzui/launchpad/adapt-subscription-to-pillar/+merge/116095
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~sinzui/launchpad/adapt-subscription-to-pillar into lp:launchpad.
=== modified file 'lib/lp/app/javascript/autocomplete/autocomplete.js'
--- lib/lp/app/javascript/autocomplete/autocomplete.js 2012-06-29 03:38:38 +0000
+++ lib/lp/app/javascript/autocomplete/autocomplete.js 2012-07-20 23:27:21 +0000
@@ -234,7 +234,8 @@
var cbox = this.get(CONTENT_BOX);
- this.get(BOUNDING_BOX).unplug(Y.Plugin.NodeMenuNav);
+ var box = this.get(BOUNDING_BOX);
+ box.unplug(Y.Plugin.NodeMenuNav);
if (this._completions) {
cbox.replaceChild(list, this._completions);
@@ -243,7 +244,8 @@
}
// Re-plug the MenuNav, so it updates the menu options.
- this.get(BOUNDING_BOX).plug(Y.Plugin.NodeMenuNav);
+ box.plug(Y.Plugin.NodeMenuNav);
+ box.setStyle('z-index', '31000');
// Highlight the first item.
this._selectItem(0, false);
=== modified file 'lib/lp/app/javascript/autocomplete/tests/test_autocomplete.js'
--- lib/lp/app/javascript/autocomplete/tests/test_autocomplete.js 2012-06-29 04:36:14 +0000
+++ lib/lp/app/javascript/autocomplete/tests/test_autocomplete.js 2012-07-20 23:27:21 +0000
@@ -448,6 +448,16 @@
Y.Event.simulate(this.input, "keydown", { keyCode: 9 });
},
+ test_pressing_matching_key_raises_menu: function() {
+ this.autocomp.set('data', ['aaaa', 'aabb']);
+ this.complete_input('aa');
+ var box = this.autocomp.get('boundingBox');
+ Assert.areEqual(
+ '31000',
+ box.getStyle('z-index'),
+ "The menu z-index should be 31000; above it's overlay.");
+ },
+
test_pressing_enter_completes_current_input: function() {
this.autocomp.set('data', ['aaaa', 'aabb']);
=== added file 'lib/lp/bugs/adapters/bugsubscriptionfilter.py'
--- lib/lp/bugs/adapters/bugsubscriptionfilter.py 1970-01-01 00:00:00 +0000
+++ lib/lp/bugs/adapters/bugsubscriptionfilter.py 2012-07-20 23:27:21 +0000
@@ -0,0 +1,26 @@
+# Copyright 2012 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Adapt IStructuralSubscription to other types."""
+
+__metaclass__ = type
+__all__ = [
+ 'bugsubscriptionfilter_to_distribution',
+ 'bugsubscriptionfilter_to_product',
+ ]
+
+
+def bugsubscriptionfilter_to_distribution(bug_subscription_filter):
+ """Adapt the `IBugSubscriptionFilter` to an `IDistribution`."""
+ subscription = bug_subscription_filter.structural_subscription
+ if subscription.distroseries is not None:
+ return subscription.distroseries.distribution
+ return subscription.distribution
+
+
+def bugsubscriptionfilter_to_product(bug_subscription_filter):
+ """Adapt the `IBugSubscriptionFilter` to an `IProduct`."""
+ subscription = bug_subscription_filter.structural_subscription
+ if subscription.productseries is not None:
+ return subscription.productseries.product
+ return subscription.product
=== added file 'lib/lp/bugs/adapters/tests/test_bugsubscriptionfilter.py'
--- lib/lp/bugs/adapters/tests/test_bugsubscriptionfilter.py 1970-01-01 00:00:00 +0000
+++ lib/lp/bugs/adapters/tests/test_bugsubscriptionfilter.py 2012-07-20 23:27:21 +0000
@@ -0,0 +1,64 @@
+# Copyright 2012 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Test IBugSubscriptionFilter adapters"""
+
+__metaclass__ = type
+
+from lp.bugs.adapters.bugsubscriptionfilter import (
+ bugsubscriptionfilter_to_distribution,
+ bugsubscriptionfilter_to_product,
+ )
+from lp.bugs.model.bugsubscriptionfilter import BugSubscriptionFilter
+from lp.registry.interfaces.distribution import IDistribution
+from lp.registry.interfaces.product import IProduct
+from lp.testing import (
+ login_person,
+ TestCaseWithFactory,
+ )
+from lp.testing.layers import DatabaseFunctionalLayer
+
+
+class BugSubscriptionFilterTestCase(TestCaseWithFactory):
+
+ layer = DatabaseFunctionalLayer
+
+ def makeBugSubscriptionFilter(self, target):
+ subscriber = self.factory.makePerson()
+ login_person(subscriber)
+ subscription = target.addBugSubscription(subscriber, subscriber)
+ subscription_filter = BugSubscriptionFilter()
+ subscription_filter.structural_subscription = subscription
+ return subscription_filter
+
+ def test_bugsubscriptionfilter_to_product_with_product(self):
+ product = self.factory.makeProduct()
+ subscription_filter = self.makeBugSubscriptionFilter(product)
+ self.assertEqual(
+ product, bugsubscriptionfilter_to_product(subscription_filter))
+ self.assertEqual(product, IProduct(subscription_filter))
+
+ def test_bugsubscriptionfilter_to_product_with_productseries(self):
+ product = self.factory.makeProduct()
+ series = product.development_focus
+ subscription_filter = self.makeBugSubscriptionFilter(series)
+ self.assertEqual(
+ product, bugsubscriptionfilter_to_product(subscription_filter))
+ self.assertEqual(product, IProduct(subscription_filter))
+
+ def test_bugsubscriptionfilter_to_distribution_with_distribution(self):
+ distribution = self.factory.makeDistribution()
+ subscription_filter = self.makeBugSubscriptionFilter(distribution)
+ self.assertEqual(
+ distribution,
+ bugsubscriptionfilter_to_distribution(subscription_filter))
+ self.assertEqual(distribution, IDistribution(subscription_filter))
+
+ def test_bugsubscriptionfilter_to_distroseries_with_distribution(self):
+ distribution = self.factory.makeDistribution()
+ series = self.factory.makeDistroSeries(distribution=distribution)
+ subscription_filter = self.makeBugSubscriptionFilter(series)
+ self.assertEqual(
+ distribution,
+ bugsubscriptionfilter_to_distribution(subscription_filter))
+ self.assertEqual(distribution, IDistribution(subscription_filter))
=== modified file 'lib/lp/bugs/configure.zcml'
--- lib/lp/bugs/configure.zcml 2012-03-08 11:51:59 +0000
+++ lib/lp/bugs/configure.zcml 2012-07-20 23:27:21 +0000
@@ -623,6 +623,14 @@
interface=".interfaces.bugsubscriptionfilter.IBugSubscriptionFilterMethodsProtected"
set_schema=".interfaces.bugsubscriptionfilter.IBugSubscriptionFilterAttributes" />
</class>
+ <adapter
+ for=".interfaces.bugsubscriptionfilter.IBugSubscriptionFilter"
+ provides="lp.registry.interfaces.distribution.IDistribution"
+ factory=".adapters.bugsubscriptionfilter.bugsubscriptionfilter_to_distribution"/>
+ <adapter
+ for=".interfaces.bugsubscriptionfilter.IBugSubscriptionFilter"
+ provides="lp.registry.interfaces.product.IProduct"
+ factory=".adapters.bugsubscriptionfilter.bugsubscriptionfilter_to_product"/>
<!-- BugMute -->
<class
=== modified file 'lib/lp/bugs/javascript/bug_tags_entry.js'
--- lib/lp/bugs/javascript/bug_tags_entry.js 2012-06-28 16:00:11 +0000
+++ lib/lp/bugs/javascript/bug_tags_entry.js 2012-07-20 23:27:21 +0000
@@ -262,14 +262,14 @@
* @method setup_tag_complete
*/
namespace.setup_tag_complete = function(input, official_tags) {
- Y.one('body').appendChild(
- '<div id="tags-autocomplete"><div id="tags-autocomplete-content">' +
- '</div></div>');
+ var bounding_box = Y.Node.create(
+ '<div class="bug-tag-complete"><div></div></div>');
+ Y.one('body').appendChild(bounding_box);
var autocomplete = new Y.lazr.AutoComplete({
input: input,
data: official_tags,
- boundingBox: '#tags-autocomplete',
- contentBox: '#tags-autocomplete-content'
+ boundingBox: bounding_box,
+ contentBox: bounding_box.one('div')
});
autocomplete.get('input').on('focus', function(e) {
autocomplete.render();
=== modified file 'lib/lp/bugs/javascript/tests/test_bug_tags_entry.js'
--- lib/lp/bugs/javascript/tests/test_bug_tags_entry.js 2012-06-28 16:15:31 +0000
+++ lib/lp/bugs/javascript/tests/test_bug_tags_entry.js 2012-07-20 23:27:21 +0000
@@ -87,7 +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'));
- Y.Assert.isInstanceOf(Y.Node, Y.one('#tags-autocomplete'));
+ Y.Assert.isInstanceOf(Y.Node, Y.one('.bug-tag-complete'));
},
test_show_activity: function() {
@@ -221,10 +221,11 @@
// The Autocompleter nodes are provided.
module.setup_tag_complete(
'input[id="field.tag"]',['project-tag']);
- var completer_node = Y.one('#tags-autocomplete');
+ var completer_node = Y.one('.yui3-autocomplete');
Y.Assert.isInstanceOf(Y.Node, completer_node);
+ Y.Assert.isTrue(completer_node.hasClass('bug-tag-complete'));
var completer_content = completer_node.one(
- '#tags-autocomplete-content');
+ '.yui3-autocomplete-content');
Y.Assert.isInstanceOf(Y.Node, completer_content);
var input = Y.one('input[id="field.tag"]');
},
=== modified file 'lib/lp/registry/javascript/structural-subscription.js'
--- lib/lp/registry/javascript/structural-subscription.js 2012-07-12 13:16:29 +0000
+++ lib/lp/registry/javascript/structural-subscription.js 2012-07-20 23:27:21 +0000
@@ -37,6 +37,14 @@
Y.augment(PortletTarget, Y.Event.Target);
namespace.portlet = new PortletTarget();
+
+/*
+ * There can be only one instance of bug_tag_completer because it is watching
+ * keyboard input.
+ */
+namespace.bug_tag_completer = null;
+
+
/**
* This helper function is used to cleanly close down the overlay.
*/
@@ -45,6 +53,9 @@
var filter_wrapper = Y.one('#' + FILTER_WRAPPER);
filter_wrapper.hide();
collapse_node(filter_wrapper);
+ if (Y.Lang.isObject(namespace.bug_tag_completer)) {
+ namespace.bug_tag_completer.destroy();
+ }
}
function subscription_success() {
@@ -116,7 +127,7 @@
// Tags are a list with one element being a space-separated string.
var tags = form_data.tags[0];
if (Y.Lang.isValue(tags) && tags !== '') {
- patch_data.tags = tags.toLowerCase().split(' ');
+ patch_data.tags = Y.Lang.trim(tags).toLowerCase().split(' ');
}
patch_data.find_all_tags =
list_contains(form_data.tag_match, MATCH_ALL);
@@ -403,6 +414,7 @@
namespace._add_subscription_overlay.on('cancel', clean_up);
}
+
/**
* Reset the overlay form to initial values.
*/
@@ -629,7 +641,9 @@
make_select_handler(node, statuses, true));
selectors.none_link.on('click',
make_select_handler(node, statuses, false));
-
+ var official_bug_tags = LP.cache.context.official_bug_tags || [];
+ namespace.bug_tag_completer = Y.lp.bugs.tags_entry.setup_tag_complete(
+ 'input[name="tags"]', official_bug_tags);
return accordion;
}
@@ -1863,5 +1877,5 @@
}, '0.1', {requires: [
'dom', 'node', 'lp.anim', 'lazr.formoverlay', 'lazr.overlay',
'lazr.effects', 'lp.app.errors', 'lp.client', 'gallery-accordion',
- 'lp.app.inlinehelp'
+ 'lp.app.inlinehelp', 'lp.bugs.tags_entry'
]});
=== modified file 'lib/lp/registry/javascript/tests/test_structural_subscription.html'
--- lib/lp/registry/javascript/tests/test_structural_subscription.html 2012-07-07 14:00:30 +0000
+++ lib/lp/registry/javascript/tests/test_structural_subscription.html 2012-07-20 23:27:21 +0000
@@ -44,7 +44,11 @@
<script type="text/javascript"
src="../../../../../build/js/lp/app/inlinehelp/inlinehelp.js"></script>
<script type="text/javascript"
- src="../../../../../build/js/lp/app//gallery-accordion/gallery-accordion.js"></script>
+ src="../../../../../build/js/lp/app/gallery-accordion/gallery-accordion.js"></script>
+ <script type="text/javascript"
+ src="../../../../../build/js/lp/app/autocomplete/autocomplete.js"></script>
+ <script type="text/javascript"
+ src="../../../../../build/js/lp/bugs/bug_tags_entry.js"></script>
<!-- The module under test. -->
<script type="text/javascript" src="../structural-subscription.js"></script>
=== modified file 'lib/lp/registry/javascript/tests/test_structural_subscription.js'
--- lib/lp/registry/javascript/tests/test_structural_subscription.js 2012-06-28 15:30:15 +0000
+++ lib/lp/registry/javascript/tests/test_structural_subscription.js 2012-07-20 23:27:21 +0000
@@ -453,6 +453,21 @@
Assert.areEqual(
'Add a mail subscription for Test bugs',
header.get('text'));
+ var bug_tags_node = Y.one(".bug-tag-complete");
+ Assert.isInstanceOf(Y.Node, bug_tags_node);
+ },
+
+ test_clean_up_overlay: function() {
+ // When the overlay is hidden, resources are cleaned up..
+ module.setup(this.configuration);
+ module._show_add_overlay(this.configuration);
+ var cancel_button = Y.one('.lazr-neg.lazr-btn');
+ var called = false;
+ module.bug_tag_completer.after('destroy', function() {
+ called = true;
+ });
+ cancel_button.simulate('click');
+ Assert.isTrue(called);
},
test_setup_overlay_missing_content_box: function() {
@@ -997,13 +1012,14 @@
name: [],
events: [],
filters: ['advanced-filter'],
- tags: ['one two THREE'],
+ tags: [' one two THREE '],
tag_match: [''],
importances: [],
statuses: []
};
var patch_data = module._extract_form_data(form_data);
- // Note that the tags are converted to lower case.
+ // Note that the tags are converted to lower case
+ // and outer white-space is stripped.
ArrayAssert.itemsAreEqual(
patch_data.tags, ['one', 'two', 'three']);
},
Follow ups
