launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #03240
[Merge] lp:~danilo/launchpad/add-subscription-link-tests into lp:launchpad
Данило Шеган has proposed merging lp:~danilo/launchpad/add-subscription-link-tests into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~danilo/launchpad/add-subscription-link-tests/+merge/56765
= Add a few tests for the 'Add a subscriber' link =
The branch that introduced 'Add a subscriber' link didn't include tests for the newly introduced functionality (such as adding the actual link, or what happens when it's clicked).
== Implementation details ==
This adds tests only for the core pieces:
- AJAXifying of the link
- Success callback for 'add subscription' which adds appropriate
filter description
In ideal world, we'd have more unit tests for all the bits and pieces, but most of the other code is tested implicitely for now, so I didn't bother improving the test coverage there as well.
== Tests ==
bin/test -cvvt structural-subscriptions.xx-advanced-features
lib/lp/registry/javascript/tests/test_structural_subscription.html
== Demo and Q/A ==
With "malone.advanced-structural-subscriptions.enabled default 1 on" added on +feature-rules, try out
https://launchpad.dev/firefox/+subscriptions
No QA is actually needed since we only introduce new tests, but this is just for the reviewer's benefit.
= Launchpad lint =
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/bugs/stories/structural-subscriptions/xx-advanced-features.txt
lib/lp/registry/javascript/tests/test_structural_subscription.js
lib/lp/registry/javascript/structural-subscription.js
./lib/lp/registry/javascript/structural-subscription.js
209: Line exceeds 78 characters.
568: Line exceeds 78 characters.
677: Line exceeds 78 characters.
715: Line exceeds 78 characters.
1072: Line exceeds 78 characters.
--
https://code.launchpad.net/~danilo/launchpad/add-subscription-link-tests/+merge/56765
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~danilo/launchpad/add-subscription-link-tests into lp:launchpad.
=== added file 'lib/lp/bugs/stories/structural-subscriptions/xx-advanced-features.txt'
--- lib/lp/bugs/stories/structural-subscriptions/xx-advanced-features.txt 1970-01-01 00:00:00 +0000
+++ lib/lp/bugs/stories/structural-subscriptions/xx-advanced-features.txt 2011-04-07 13:28:39 +0000
@@ -0,0 +1,26 @@
+Advanced structural subscriptions
+---------------------------------
+
+There are advanced features for structural bug subscriptions.
+These require a feature flag to be turned on for them to be accessible.
+
+ >>> from lp.services.features.model import FeatureFlag, getFeatureStore
+ >>> feature_store = getFeatureStore()
+ >>> ignore = feature_store.add(FeatureFlag(
+ ... scope=u'default',
+ ... flag=u'malone.advanced-structural-subscriptions.enabled',
+ ... value=u'on', priority=1))
+ >>> feature_store.flush()
+
+Now when a user visits the +subscriptions page of a product they are given the
+option to subscribe to add a new subscription
+
+ >>> from canonical.launchpad.webapp import canonical_url
+ >>> from lp.testing.sampledata import USER_EMAIL
+ >>> login(USER_EMAIL)
+ >>> product = factory.makeProduct()
+ >>> logout()
+ >>> user_browser.open(
+ ... canonical_url(product, view_name='+subscriptions'))
+ >>> user_browser.getLink("Add a subscription")
+ <Link text='Add a subscription' url='.../+subscriptions#'>
=== modified file 'lib/lp/registry/javascript/structural-subscription.js'
--- lib/lp/registry/javascript/structural-subscription.js 2011-04-07 02:14:43 +0000
+++ lib/lp/registry/javascript/structural-subscription.js 2011-04-07 13:28:39 +0000
@@ -1484,6 +1484,8 @@
}
}
}
+namespace._make_add_subscription_success_handler =
+ make_add_subscription_success_handler;
/**
* Show the overlay for creating a new subscription.
@@ -1514,23 +1516,28 @@
namespace._show_add_overlay = show_add_overlay;
/**
- * Modify a link to pop up a subscription overlay.
+ * Modify a link to pop up a subscription overlay on click.
*
* @method setup_subscription_link
+ * @param {Object} config Overlay configuration object.
* @param {String} link_id Id of the link element.
- * @param {String} overlay_id Id of the overlay element.
*/
function setup_subscription_link(config, link_id) {
// Modify the menu-link-subscribe-to-bug-mail link to be visible.
var link = Y.one(link_id);
if (!Y.Lang.isValue(link)) {
+<<<<<<< TREE
Y.error('Link to set as the pop-up link not found.');
+=======
+ throw new Error('Link to set as the pop-up link not found.');
+>>>>>>> MERGE-SOURCE
}
link.removeClass('invisible-link');
link.addClass('visible-link');
link.on('click', function(e) {
e.halt();
- show_add_overlay(config);
+ // Call with the namespace so tests can override it.
+ namespace._show_add_overlay(config);
});
link.addClass('js-action');
} // setup_subscription_links
@@ -1554,7 +1561,13 @@
}; // setup
}, '0.1', {requires: [
+<<<<<<< TREE
'dom', 'node', 'lazr.anim', 'lazr.formoverlay',
'lazr.overlay','lazr.effects', 'lp.app.errors', 'lp.client',
'gallery-accordion'
]});
+=======
+ 'dom', 'node', 'lazr.anim', 'lazr.formoverlay', 'lazr.overlay',
+ 'lazr.effects', 'lp.app.errors', 'lp.client', 'gallery-accordion'
+]});
+>>>>>>> MERGE-SOURCE
=== modified file 'lib/lp/registry/javascript/tests/test_structural_subscription.js'
--- lib/lp/registry/javascript/tests/test_structural_subscription.js 2011-04-07 02:14:43 +0000
+++ lib/lp/registry/javascript/tests/test_structural_subscription.js 2011-04-07 13:28:39 +0000
@@ -808,6 +808,169 @@
}));
+ suite.add(new Y.Test.Case({
+ name: 'Add a subscription from +subscriptions page',
+
+ setUp: function() {
+ this.config = {
+ content_box: content_box_id
+ }
+ this.content_box = create_test_node();
+ Y.one('body').appendChild(this.content_box);
+ },
+
+ tearDown: function() {
+ //delete this.configuration;
+ remove_test_node();
+ delete this.content_box;
+ },
+
+ _should: {
+ error: {
+ test_setup_subscription_link_none: new Error(
+ 'Link to set as the pop-up link not found.')
+ }
+ },
+
+ // Setting up a subscription link with no link in the DOM should fail.
+ test_setup_subscription_link_none: function() {
+ module.setup_subscription_link(this.config, "#link");
+ },
+
+ // Setting up a subscription link should unset the 'invisible-link',
+ // and set 'visible-link' and 'js-action' CSS classes on the node.
+ test_setup_subscription_link_classes: function() {
+ var link = this.content_box.appendChild(
+ Y.Node.create('<a>Link</a>'));
+ link.set('id', 'link');
+ link.addClass('invisible-link');
+ module.setup_subscription_link(this.config, "#link");
+ Assert.isFalse(link.hasClass('invisible-link'));
+ Assert.isTrue(link.hasClass('visible-link'));
+ Assert.isTrue(link.hasClass('js-action'));
+ },
+
+ // Setting up a subscription link creates an on-click handler
+ // that calls up show_add_overlay with the passed in configuration.
+ test_setup_subscription_link_behaviour: function() {
+ var link = this.content_box.appendChild(
+ Y.Node.create('<a>Link</a>'));
+ link.set('id', 'link');
+
+ // Track if the method was called.
+ var called_method = false;
+
+ // Keep the old module's _show_add_overlay, so we can override.
+ old_show_add_overlay = module._show_add_overlay;
+ var test = this;
+ module._show_add_overlay = function(config) {
+ Assert.areEqual(test.config, config);
+ called_method = true;
+ module._show_add_overlay = old_show_add_overlay;
+ }
+ module.setup_subscription_link(this.config, "#link");
+ Y.Event.simulate(Y.Node.getDOMNode(link), 'click');
+
+ this.wait(function() {
+ Assert.isTrue(called_method);
+ }, 20);
+ },
+
+ // Success handler for adding a subscription does nothing in
+ // the DOM if config.add_filter_description is not set.
+ test_make_add_subscription_success_handler_nothing: function() {
+ var success_handler =
+ module._make_add_subscription_success_handler(this.config);
+ var subs_list = this.content_box.appendChild(
+ Y.Node.create('<div id="structural-subscriptions"></div>'));
+ success_handler();
+ // No sub-nodes have been created in the subs_list node.
+ Assert.isTrue(subs_list.all('div.subscription-filter').isEmpty());
+ },
+
+ // Success handler for adding a subscription creates
+ // a subscription listing if there's none and adds a filter to it.
+ test_make_add_subscription_success_handler_empty_list: function() {
+ this.config.add_filter_description = true;
+ var success_handler =
+ module._make_add_subscription_success_handler(this.config);
+ var subs_list = this.content_box.appendChild(
+ Y.Node.create('<div id="structural-subscriptions"></div>'));
+
+ var form_data = {
+ recipient: ["user"],
+ };
+ var target_info = {
+ title: "MY TARGET",
+ url: "http://target/"; };
+ window.LP.cache.target_info = target_info;
+ var filter = {
+ getAttrs: function() {
+ return {
+ importances: [],
+ statuses: [],
+ tags: [],
+ find_all_tags: true,
+ bug_notification_level: 'Discussion',
+ description: 'Filter name'
+ };
+ }};
+
+ success_handler(form_data, filter);
+ // No sub-nodes have been created in the subs_list node.
+ Assert.areEqual(
+ 1, subs_list.all('div.subscription-filter').size());
+ var target_node = subs_list.one('#subscription-0>span>span');
+ Assert.areEqual(
+ 'Subscriptions to MY TARGET',
+ subs_list.one('#subscription-0>span>span').get('text'));
+ var filter_node = subs_list.one('#subscription-filter-0');
+ Assert.areEqual(
+ 'Your subscription: "Filter name"',
+ filter_node.one('.filter-name').get('text'));
+ this.config.add_filter_description = false;
+ delete window.LP.cache.target_info;
+ },
+
+ // Success handler for adding a subscription adds a filter
+ // to the subscription listing which already has filters listed.
+ test_make_add_subscription_success_handler_with_filters: function() {
+ this.config.add_filter_description = true;
+ var success_handler =
+ module._make_add_subscription_success_handler(this.config);
+ var subs_list = this.content_box.appendChild(
+ Y.Node.create('<div id="structural-subscriptions"></div>'));
+ subs_list.appendChild('<div id="subscription-0"></div>')
+ .appendChild('<div id="subscription-filter-0"'+
+ ' class="subscription-filter"></div>');
+ var form_data = {
+ recipient: ["user"],
+ };
+ var target_info = {
+ title: "Subscription target",
+ url: "http://target/"; };
+ window.LP.cache.target_info = target_info;
+ var filter = {
+ getAttrs: function() {
+ return {
+ importances: [],
+ statuses: [],
+ tags: [],
+ find_all_tags: true,
+ bug_notification_level: 'Discussion',
+ description: 'Filter name'
+ };
+ }};
+
+ success_handler(form_data, filter);
+ // No sub-nodes have been created in the subs_list node.
+ Assert.areEqual(
+ 2, subs_list.all('div.subscription-filter').size());
+ this.config.add_filter_description = false;
+ delete window.LP.cache.target_info;
+ },
+ }));
+
// Lock, stock, and two smoking barrels.
var handle_complete = function(data) {
var status_node = Y.Node.create(
