launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #03485
[Merge] lp:~benji/launchpad/bug-771231 into lp:launchpad
Benji York has proposed merging lp:~benji/launchpad/bug-771231 into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~benji/launchpad/bug-771231/+merge/59555
Bug 771231 is about getting confirmation that adding a structural
subscription worked and that the new subscription is configured the way
the user intended. This branch adds such a confirmation by building an
informational message box (the same kind of box
response.addNotification(text) results in).
The related JS tests can be run by loading
lib/lp/registry/javascript/tests/test_structural_subscription.html in a
browser. They all pass locally.
The make lint report is clean.
--
https://code.launchpad.net/~benji/launchpad/bug-771231/+merge/59555
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~benji/launchpad/bug-771231 into lp:launchpad.
=== modified file 'lib/lp/registry/javascript/structural-subscription.js'
--- lib/lp/registry/javascript/structural-subscription.js 2011-04-28 17:07:38 +0000
+++ lib/lp/registry/javascript/structural-subscription.js 2011-04-29 21:10:54 +0000
@@ -1567,6 +1567,36 @@
config, subscription_info, 0,
filter_info, filter_id, anim_node);
Y.lazr.anim.green_flash({node: anim_node}).run();
+ } else {
+ // Since there is no filter description to update we need another
+ // way to tell the user that the subscription was sucessfully
+ // added. We'll do that by creating an informational message box.
+ var description = filter.get('description');
+ var header = Y.Node.create('<div/>')
+ .setStyle('marginBottom', '1em')
+ .append('The subscription')
+ .append(' '); // a space
+ if (description) {
+ header
+ .append('named "')
+ .append(Y.Node.create('<span/>')
+ .set('text', description))
+ .append('"')
+ .append(' '); // a space
+ }
+ header.append('has been created.');
+
+ Y.one('#request-notifications')
+ .empty()
+ .append(Y.Node.create('<div/>')
+ // We're creating an informational message box.
+ .addClass('informational')
+ .addClass('message')
+ // The box needs to be a little wider to accomodate the
+ // wordy subscription description.
+ .setStyle('width', '50em')
+ .append(header)
+ .append(create_filter_description(filter.getAttrs())));
}
};
}
=== modified file 'lib/lp/registry/javascript/tests/test_structural_subscription.html'
--- lib/lp/registry/javascript/tests/test_structural_subscription.html 2011-04-11 16:12:18 +0000
+++ lib/lp/registry/javascript/tests/test_structural_subscription.html 2011-04-29 21:10:54 +0000
@@ -55,6 +55,7 @@
</style>
</head>
<body class="yui3-skin-sam">
+ <div id="request-notifications"></div>
<div id="log"></div>
</body>
</html>
=== modified file 'lib/lp/registry/javascript/tests/test_structural_subscription.js'
--- lib/lp/registry/javascript/tests/test_structural_subscription.js 2011-04-27 22:00:21 +0000
+++ lib/lp/registry/javascript/tests/test_structural_subscription.js 2011-04-29 21:10:54 +0000
@@ -907,8 +907,18 @@
setUp: function() {
var TestBugFilter = function() {};
TestBugFilter.prototype = {
- 'getAttrs': function () {
- return {};
+ get: function (name) {
+ return 'DESCRIPTION';
+ },
+ getAttrs: function () {
+ return {
+ description: 'DESCRIPTION',
+ statuses: [],
+ importances: [],
+ tags: [],
+ find_all_tags: true,
+ bug_notification_level: 'Discussion'
+ };
}
};
// We need an lp_client that will appear to succesfully create the
@@ -920,7 +930,7 @@
this.post_called = true;
},
patch: function(uri, representation, config, headers) {
- config.on.success();
+ config.on.success(new TestBugFilter());
this.patch_called = true;
},
post_called: false,
@@ -977,6 +987,49 @@
}));
suite.add(new Y.Test.Case({
+ // The make_add_subscription_success_handler function constructs a
+ // function that gives a visual feedback after adding a subscription.
+
+ name:
+ 'Structural Subscription: make_add_subscription_success_handler',
+
+ _should: {error: {}},
+
+ setUp: function() {
+ this.TestBugFilter = function() {};
+ this.TestBugFilter.prototype = {
+ get: function (name) {
+ return 'DESCRIPTION';
+ },
+ getAttrs: function () {
+ return {
+ description: 'DESCRIPTION',
+ statuses: [],
+ importances: [],
+ tags: [],
+ find_all_tags: true,
+ bug_notification_level: 'Discussion'
+ };
+ }
+ };
+ },
+
+ test_description_is_added: function() {
+ // If we add a subscription on a page that doesn't display
+ // subcription details then we need to add an "informational
+ // message" describing the just-added subscription.
+ var handler;
+ var config = {add_filter_description: false};
+ handler = module._make_add_subscription_success_handler(config);
+ var form_data = {};
+ handler(form_data, new this.TestBugFilter());
+ var text = Y.one('#request-notifications').get('text');
+ Assert.isTrue(text.indexOf('DESCRIPTION') !== -1);
+ }
+
+ }));
+
+ suite.add(new Y.Test.Case({
name: 'Structural Subscription: edit subcription workflow',
_should: {error: {}},
@@ -1198,10 +1251,11 @@
};
this.content_box = create_test_node();
Y.one('body').appendChild(this.content_box);
+ this.original_lp = monkeypatch_LP();
},
tearDown: function() {
- //delete this.configuration;
+ window.LP = this.original_lp;
remove_test_node();
delete this.content_box;
},
@@ -1257,18 +1311,6 @@
}, 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() {
@@ -1286,6 +1328,9 @@
url: "http://target/"; };
window.LP.cache.target_info = target_info;
var filter = {
+ get: function (name) {
+ return 'DESCRIPTION';
+ },
getAttrs: function() {
return {
importances: [],
@@ -1330,6 +1375,27 @@
var target_info = {
title: "Subscription target",
url: "http://target/"; };
+ window.LP.cache.subscription_info = [{
+ target_url: 'http://example.com',
+ target_title:'Example project',
+ filters: [{
+ filter: {
+ description: 'DESCRIPTION',
+ statuses: [],
+ importances: [],
+ tags: [],
+ find_all_tags: true,
+ bug_notification_level: 'Discussion',
+ self_link: 'http://example.com/a_filter'
+ },
+ can_mute: true,
+ is_muted: false,
+ subscriber_is_team: false,
+ subscriber_url: 'http://example.com/subscriber',
+ subscriber_title: 'Thidwick',
+ user_is_team_admin: false
+ }]
+ }];
window.LP.cache.target_info = target_info;
var filter = {
getAttrs: function() {
