launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #03202
[Merge] lp:~danilo/launchpad/add-subscription-link into lp:launchpad
Данило Шеган has proposed merging lp:~danilo/launchpad/add-subscription-link into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~danilo/launchpad/add-subscription-link/+merge/56392
= Provide 'add subscription' from target:+subscriptions =
Allow creating a target subscription from the subscriptions overview page.
Lots of refactoring of the existing code (mostly moving stuff around and generalizing so we can get all the data we need and render the stuff with the same code).
== Tests ==
No tests atm. Will do them as a separate branch.
== Demo and Q/A ==
https://launchpad.dev/firefox/+subscriptions
https://launchpad.dev/firefox/ ("Subscribe to bug mail" link keeps working)
= Launchpad lint =
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/bugs/browser/structuralsubscription.py
lib/lp/bugs/templates/bugtarget-subscription-list.pt
lib/lp/registry/javascript/structural-subscription.js
./lib/lp/registry/javascript/structural-subscription.js
209: Line exceeds 78 characters.
672: Line exceeds 78 characters.
710: Line exceeds 78 characters.
--
https://code.launchpad.net/~danilo/launchpad/add-subscription-link/+merge/56392
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~danilo/launchpad/add-subscription-link into lp:launchpad.
=== modified file 'lib/lp/bugs/browser/structuralsubscription.py'
--- lib/lp/bugs/browser/structuralsubscription.py 2011-04-01 20:26:38 +0000
+++ lib/lp/bugs/browser/structuralsubscription.py 2011-04-05 15:35:46 +0000
@@ -382,10 +382,13 @@
expose_user_administered_teams_to_js(request, user, context)
expose_enum_to_js(request, BugTaskImportance, 'importances')
expose_enum_to_js(request, BugTaskStatus, 'statuses')
- if subscriptions is None:
+ if subscriptions is None or subscriptions.is_empty():
subscriptions = []
+ target = context
+ else:
+ target = None
expose_user_subscriptions_to_js(
- user, subscriptions, request)
+ user, subscriptions, request, target)
def expose_enum_to_js(request, enum, name):
@@ -413,11 +416,12 @@
info.append({
'link': absoluteURL(team, api_request),
'title': team.title,
+ 'url': canonical_url(team),
})
IJSONRequestCache(request).objects['administratedTeams'] = info
-def expose_user_subscriptions_to_js(user, subscriptions, request):
+def expose_user_subscriptions_to_js(user, subscriptions, request, target):
"""Make the user's subscriptions available to JavaScript."""
info = {}
api_request = IWebServiceClientRequest(request)
@@ -425,6 +429,15 @@
administered_teams = []
else:
administered_teams = user.getAdministratedTeams()
+
+ target_info = {}
+ if target is not None:
+ # No subscriptions, which means we are on a target subscriptions page.
+ # Let's at least provide target details.
+ target_info['title'] = target.title
+ target_info['url'] = canonical_url(target, rootsite='mainsite')
+ IJSONRequestCache(request).objects['target_info'] = target_info
+
for subscription in subscriptions:
target = subscription.target
record = info.get(target)
=== modified file 'lib/lp/bugs/templates/bugtarget-subscription-list.pt'
--- lib/lp/bugs/templates/bugtarget-subscription-list.pt 2011-03-29 22:34:04 +0000
+++ lib/lp/bugs/templates/bugtarget-subscription-list.pt 2011-04-05 15:35:46 +0000
@@ -17,9 +17,13 @@
request/features/malone.advanced-structural-subscriptions.enabled">
LPS.use('lp.registry.structural_subscription', function(Y) {
var module = Y.lp.registry.structural_subscription;
+ var config = {
+ content_box: "#structural-subscription-content-box",
+ add_filter_description: true};
Y.on('domready', function() {
- module.setup_bug_subscriptions(
- {content_box: "#structural-subscription-content-box"})
+ module.setup_bug_subscriptions(config);
+ module.setup_subscription_link(
+ config, '#create-new-subscription');
});
});
</script>
@@ -31,6 +35,12 @@
<div id="maincontent">
<div id="nonportlets" class="readable">
+ <div tal:condition="
+ features/malone.advanced-structural-subscriptions.enabled">
+ <a class="sprite add" id="create-new-subscription"
+ href="#">Add a subscription</a>
+ </div>
+
<div id="subscription-listing"></div>
<div id="structural-subscription-content-box"></div>
=== modified file 'lib/lp/registry/javascript/structural-subscription.js'
--- lib/lp/registry/javascript/structural-subscription.js 2011-04-04 16:22:20 +0000
+++ lib/lp/registry/javascript/structural-subscription.js 2011-04-05 15:35:46 +0000
@@ -164,20 +164,27 @@
* @method create_structural_subscription filter
* @param {Object} who Link to the user or team to be subscribed.
* @param {Object} form_data The data returned from the form submission.
+ * @param {Object} success_callback Function to execute when filter is added.
*/
-function add_bug_filter(who, form_data) {
+function add_bug_filter(who, form_data, success_callback) {
var config = {
on: {success: function (bug_filter) {
// If we fail to PATCH the new bug filter, DELETE it.
- var on = {failure: function () {
- // We use the namespace binding so tests can override
- // these functions.
- namespace._delete_filter(bug_filter);
- // Call the failure handler to report the original
- // error to the user.
- overlay_error_handler.getFailureHandler()
- .apply(this, arguments);
- }};
+ var on = {
+ failure: function () {
+ // We use the namespace binding so tests can override
+ // these functions.
+ namespace._delete_filter(bug_filter);
+ // Call the failure handler to report the original
+ // error to the user.
+ overlay_error_handler.getFailureHandler()
+ .apply(this, arguments);
+ },
+ success: function (bug_filter) {
+ success_callback(form_data, bug_filter);
+ subscription_success(bug_filter);
+ }
+ };
patch_bug_filter(bug_filter.getAttrs(), form_data, on);
},
failure: overlay_error_handler.getFailureHandler()
@@ -195,28 +202,30 @@
namespace._add_bug_filter = add_bug_filter;
/**
- * Given the form data from a user, save the subscription.
+ * Create a handler to save the subscription given the form data from a user.
*
* @private
- * @method save_subscription
- * @param {Object} form_data The data generated by the form submission.
+ * @method make_add_subscription_handler
+ * @param {Object} success_callback Function to execute on successful addition.
*/
-
-function save_subscription(form_data) {
- var who;
- var has_errors = check_for_errors_in_overlay(add_subscription_overlay);
- if (has_errors) {
- return false;
- }
- if (form_data.recipient[0] === 'user') {
- who = LP.links.me;
- } else {
- // There can be only one.
- who = form_data.team[0];
- }
- add_bug_filter(who, form_data);
+function make_add_subscription_handler(success_callback) {
+ var save_subscription = function(form_data) {
+ var who;
+ var has_errors = check_for_errors_in_overlay(
+ add_subscription_overlay);
+ if (has_errors) {
+ return false;
+ }
+ if (form_data.recipient[0] === 'user') {
+ who = LP.links.me;
+ } else {
+ // There can be only one.
+ who = form_data.team[0];
+ }
+ return add_bug_filter(who, form_data, success_callback);
+ };
+ return save_subscription;
}
-namespace.save_subscription = save_subscription;
function check_for_errors_in_overlay(overlay) {
var has_errors = false;
@@ -245,6 +254,18 @@
}
/**
+ * Fill the filter name and description.
+ */
+function fill_filter_description(description_node, filter_info, filter) {
+ description_node
+ .empty()
+ .appendChild(create_filter_description(filter));
+ description_node.ancestor('.subscription-filter').one('.filter-name')
+ .empty()
+ .appendChild(render_filter_title(filter_info, filter));
+};
+
+/**
* Handle the activation of the edit subscription link.
*/
function edit_subscription_handler(context, form_data) {
@@ -254,14 +275,10 @@
return false;
}
var on = {success: function (new_data) {
+ var description_node = Y.one(filter_id);
var filter = new_data.getAttrs();
- var description_node = Y.one(filter_id);
- description_node
- .empty()
- .appendChild(create_filter_description(filter));
- description_node.ancestor('.subscription-filter').one('.filter-name')
- .empty()
- .appendChild(render_filter_title(context.filter_info, filter));
+ fill_filter_description(
+ description_node, context.filter_info, filter);
add_subscription_overlay.hide();
}};
patch_bug_filter(context.filter_info.filter, form_data, on);
@@ -279,7 +296,7 @@
* Populate the overlay element with the contents of the add/edit form.
*/
function create_overlay(content_box_id, overlay_id, submit_button,
- submit_callback) {
+ submit_callback, success_callback) {
// Create the overlay.
add_subscription_overlay = new Y.lazr.FormOverlay({
headerContent:
@@ -881,13 +898,8 @@
var recipient_label = content_node.one('input[name="recipient"] + span'),
teams = LP.cache.administratedTeams;
if (filter_info !== undefined && filter_info.subscriber_is_team) {
- var i;
- for (i=0; i<teams.length; i++) {
- if (teams[i].link === filter_info.subscriber_link){
- recipient_label.set('text', teams[i].title);
- break;
- }
- }
+ var team = get_team(filter_info.subscriber_link);
+ recipient_label.set('text', team.title);
} else {
recipient_label.set('text', 'Yourself');
}
@@ -1049,8 +1061,9 @@
to_collapse = Y.one(
'#subscription-filter-'+filter_id.toString());
}
+ to_collapse.setStyle("margin-top", "0");
collapse_node(to_collapse);
- },
+ },
failure: error_handler.getFailureHandler()
}
};
@@ -1059,6 +1072,26 @@
}
/**
+ * Attach activation (click) handlers to links for a particular filter.
+ */
+function wire_up_edit_links_for_filter(
+ config, subscription, subscription_id, filter_info, filter_id) {
+ if (!filter_info.subscriber_is_team ||
+ filter_info.user_is_team_admin) {
+ var node = Y.one(
+ '#subscription-filter-'+filter_id.toString());
+ var edit_link = node.one('a.edit-subscription');
+ var edit_handler = make_edit_handler(
+ subscription, filter_info, filter_id, config);
+ edit_link.on('click', edit_handler);
+ var delete_link = node.one('a.delete-subscription');
+ var delete_handler = make_delete_handler(
+ filter_info.filter, filter_id, subscription_id);
+ delete_link.on('click', delete_handler);
+ }
+}
+
+/**
* Attach activation (click) handlers to all of the edit links on the page.
*/
function wire_up_edit_links(config) {
@@ -1071,25 +1104,97 @@
var sub = subscription_info[i];
for (j=0; j<sub.filters.length; j++) {
var filter_info = sub.filters[j];
- if (!filter_info.subscriber_is_team ||
- filter_info.user_is_team_admin) {
- var node = Y.one(
- '#subscription-filter-'+filter_id.toString());
- var edit_link = node.one('a.edit-subscription');
- var edit_handler = make_edit_handler(
- sub, filter_info, filter_id, config);
- edit_link.on('click', edit_handler);
- var delete_link = node.one('a.delete-subscription');
- var delete_handler = make_delete_handler(
- filter_info.filter, filter_id, i);
- delete_link.on('click', delete_handler);
- }
+ wire_up_edit_links_for_filter(
+ config, sub, i, filter_info, filter_id);
filter_id += 1;
}
}
}
/**
+ * Create filter node to include in the subscription's filter listing.
+ */
+function create_filter_node(filter_id, filter_info, filter) {
+ var filter_node = Y.Node.create(
+ '<div style="margin: 1em 0em 0em 1em"'+
+ ' class="subscription-filter"></div>')
+ .set('id', 'subscription-filter-'+filter_id.toString());
+ filter_node.appendChild(Y.Node.create(
+ '<div style="margin-top: 1em"></div>'));
+ filter_node.appendChild(Y.Node.create(
+ '<strong class="filter-name"></strong>'));
+
+ if (!filter_info.subscriber_is_team ||
+ filter_info.user_is_team_admin) {
+ // User can edit the subscription.
+ filter_node.appendChild(Y.Node.create(
+ '<span style="float: right">'+
+ '<a href="#" class="sprite modify edit js-action '+
+ ' edit-subscription">'+
+ ' Edit this subscription</a> or '+
+ '<a href="#" class="sprite modify remove js-action '+
+ ' delete-subscription">'+
+ ' Unsubscribe</a></span>'));
+ } else {
+ // User cannot edit the subscription, because this is a
+ // team and the user does not have admin privileges.
+ filter_node.appendChild(Y.Node.create(
+ '<span style="float: right"><em>'+
+ 'You do not have privileges to change this subscription'+
+ '</em></span>'));
+ }
+
+ var description_node = filter_node.appendChild(
+ Y.Node.create(
+ '<div style="padding-left: 1em"></div>')
+ .set('id', 'filter-description-'+filter_id.toString()));
+
+ fill_filter_description(description_node, filter_info, filter);
+
+ return filter_node;
+}
+
+/**
+ * Create a node with subscription description.
+ */
+function create_subscription_node(serial_id, subscription_data) {
+ var filter_id = 0;
+ var node = Y.Node.create(
+ '<div style="margin-top: 2em; padding: 0 1em 1em 1em; '+
+ ' border: 1px solid #ddd;"></div>')
+ .set('id', 'subscription-'+serial_id.toString())
+ node.appendChild(Y.Node.create(
+ ' <span style="float: left; margin-top: -0.6em; '+
+ ' padding: 0 1ex; background-color: #fff;"></a>'))
+ .appendChild('<span>Subscriptions to </span>')
+ .appendChild(Y.Node.create('<a></a>')
+ .set('href', subscription_data.target_url)
+ .set('text', subscription_data.target_title));
+
+ // We can remove this once we enforce at least one filter per
+ // subscription.
+ if (subscription_data.filters.length === 0) {
+ node.appendChild(
+ '<div style="clear: both; padding: 1em 0 0 1em"></div>')
+ .appendChild('<strong>All messages</strong>');
+ }
+
+ for (j=0; j<subscription_data.filters.length; j++) {
+ var filter_info = subscription_data.filters[j];
+ var filter = filter_info.filter;
+ // We put the filters in the cache so that the patch mechanism
+ // can automatically find them and update them on a successful
+ // edit. This makes it possible to open up a filter after an edit
+ // and see the information you expect to see.
+ LP.cache['structural-subscription-filter-'+filter_id.toString()] =
+ filter;
+ node.appendChild(create_filter_node(filter_id, filter_info, filter));
+ filter_id += 1;
+ };
+ return node;
+}
+
+/**
* Populate the subscription list DOM element with subscription descriptions.
*/
function fill_in_bug_subscriptions(config) {
@@ -1099,77 +1204,13 @@
var subscription_info = LP.cache.subscription_info;
var top_node = Y.Node.create(
'<div class="yui-g"><div id="structural-subscriptions"></div></div>');
- var filter_id = 0;
var i;
var j;
for (i=0; i<subscription_info.length; i++) {
- var sub = subscription_info[i];
- var sub_node = top_node.appendChild(Y.Node.create(
- '<div style="margin-top: 2em; padding: 0 1em 1em 1em; '+
- ' border: 1px solid #ddd;"></div>')
- .set('id', 'subscription-'+i.toString()));
- sub_node.appendChild(Y.Node.create(
- ' <span style="float: left; margin-top: -0.6em; '+
- ' padding: 0 1ex; background-color: #fff;"></a>'))
- .appendChild('<span>Subscriptions to </span>')
- .appendChild(Y.Node.create('<a></a>')
- .set('href', sub.target_url)
- .set('text', sub.target_title));
-
- for (j=0; j<sub.filters.length; j++) {
- var filter = sub.filters[j].filter;
- // We put the filters in the cache so that the patch mechanism
- // can automatically find them and update them on a successful
- // edit. This makes it possible to open up a filter after an edit
- // and see the information you expect to see.
- LP.cache['structural-subscription-filter-'+filter_id.toString()] =
- filter;
- var filter_node = sub_node.appendChild(Y.Node.create(
- '<div style="margin: 1em 0em 0em 1em"'+
- ' class="subscription-filter"></div>')
- .set('id', 'subscription-filter-'+filter_id.toString()))
- .appendChild(Y.Node.create(
- '<div style="margin-top: 1em"></div>'));
- filter_node.appendChild(Y.Node.create(
- '<strong class="filter-name"></strong>'))
- .appendChild(render_filter_title(sub.filters[j], filter));
-
- if (!sub.filters[j].subscriber_is_team ||
- sub.filters[j].user_is_team_admin) {
- // User can edit the subscription.
- filter_node.appendChild(Y.Node.create(
- '<span style="float: right">'+
- '<a href="#" class="sprite modify edit js-action '+
- ' edit-subscription">'+
- ' Edit this subscription</a> or '+
- '<a href="#" class="sprite modify remove js-action '+
- ' delete-subscription">'+
- ' Unsubscribe</a></span>'));
- } else {
- // User cannot edit the subscription, because this is a
- // team and the user does not have admin privileges.
- filter_node.appendChild(Y.Node.create(
- '<span style="float: right"><em>'+
- 'You do not have privileges to change this subscription'+
- '</em></span>'));
- }
-
- filter_node.appendChild(Y.Node.create(
- '<div style="padding-left: 1em"></div>')
- .set('id', 'filter-description-'+filter_id.toString()))
- .appendChild(create_filter_description(filter));
-
- filter_id += 1;
- }
-
- // We can remove this once we enforce at least one filter per
- // subscription.
- if (subscription_info[i].filters.length === 0) {
- sub_node.appendChild(
- '<div style="clear: both; padding: 1em 0 0 1em"></div>')
- .appendChild('<strong>All messages</strong>');
- }
+ top_node.appendChild(
+ create_subscription_node(i, subscription_info[i]));
}
+
listing.appendChild(top_node);
wire_up_edit_links(config);
@@ -1203,7 +1244,6 @@
*/
function create_filter_description(filter) {
var description = Y.Node.create('<div></div>');
-
var filter_items = [];
// Format status conditions.
if (filter.statuses.length !== 0) {
@@ -1303,20 +1343,120 @@
}
/**
+ * Get team information.
+ */
+function get_team(url) {
+ var teams = LP.cache.administratedTeams;
+ var i;
+ for (i=0; i<teams.length; i++) {
+ if (teams[i].link === url) {
+ return teams[i];
+ }
+ }
+}
+
+/**
+ * Get team information from the submitted form data.
+ */
+function get_team_info(form_data) {
+ var is_team = (form_data.recipient[0] == "team");
+ var link, team, title, url;
+ if (is_team) {
+ link = form_data.team[0];
+ team = get_team(link);
+ if (team !== undefined) {
+ title = team.title;
+ url = team.url;
+ } else {
+ is_team = false;
+ link = LP.links.me;
+ }
+ }
+ return {
+ is_team: is_team,
+ link: link,
+ title: title,
+ url: url}
+}
+
+function get_target_info() {
+ if (LP.cache.target_info !== undefined) {
+ return LP.cache.target_info
+ } else {
+ var info = LP.cache.subscription_info[0];
+ return {
+ title: info.target_title,
+ url: info.target_url}
+ }
+}
+
+/**
* Show the overlay for creating a new subscription.
*/
function show_add_overlay(config) {
- Y.one(config.content_box).empty();
+ var content_node = Y.one(config.content_box);
+ content_node.empty();
var overlay_id = setup_overlay(config.content_box);
- clear_overlay(Y.one(config.content_box), false);
+ clear_overlay(content_node, false);
var submit_button = Y.Node.create(
'<button type="submit" name="field.actions.create" ' +
'value="Create subscription" class="lazr-pos lazr-btn" '+
'>OK</button>');
+ var success_callback;
+ success_callback = function(form_data, filter) {
+ if (config.add_filter_description === true) {
+ // This way to figure out the ID works only
+ // if the page shows one "target" (eg. Firefox).
+ var subscription_info;
+ var filter_id;
+ if (LP.cache.subscription_info.length == 0) {
+ LP.cache.subscription_info.push({filters: []});
+ }
+ subscription_info = LP.cache.subscription_info[0];
+ filter_id = subscription_info.filters.length;
+ var target_info = get_target_info();
+ var subscriptions_list = Y.one('#subscription-0');
+ var filter_data = filter.getAttrs();
+ var team_info = get_team_info(form_data);
+ var filter_info = {
+ filter : filter_data,
+ subscriber_is_team: team_info.is_team,
+ user_is_team_admin: team_info.is_team,
+ subscriber_url: team_info.url,
+ subscriber_link: team_info.link,
+ subscriber_title: team_info.title,
+ target_title: target_info.title,
+ target_url: target_info.url
+ };
+ subscription_info.filters.push(filter);
+ var anim_node;
+ if (subscriptions_list === null) {
+ var sub_info = {
+ filters: [filter_info],
+ target_url: target_info.url,
+ target_title: target_info.title
+ };
+ subscriptions_list = create_subscription_node(0, sub_info);
+ Y.one("#structural-subscriptions").appendChild(
+ subscriptions_list);
+ anim_node = subscriptions_list;
+ } else {
+ var description_node = create_filter_node(
+ filter_id, filter_info, filter_data);
+ subscriptions_list.append(description_node);
+ anim_node = description_node;
+ }
+ wire_up_edit_links_for_filter(
+ config, subscription_info, 0, filter_info, filter_id);
+ Y.lazr.anim.green_flash({node: anim_node}).run();
+ }
+ }
+
+ var save_subscription = make_add_subscription_handler(success_callback);
create_overlay(config.content_box, overlay_id, submit_button,
- save_subscription);
+ save_subscription, success_callback);
// We need to initialize the help links. They may have already been
// initialized except for the ones we added, so setupHelpTrigger
// is idempotent. Notice that this is old MochiKit code.
@@ -1337,7 +1477,7 @@
// Modify the menu-link-subscribe-to-bug-mail link to be visible.
var link = Y.one(link_id);
if (!Y.Lang.isValue(link)) {
- Y.fail('Link to set as the pop-up link not found.');
+ Y.Assert.fail('Link to set as the pop-up link not found.');
}
link.removeClass('invisible-link');
link.addClass('visible-link');
@@ -1347,6 +1487,7 @@
});
link.addClass('js-action');
} // setup_subscription_links
+namespace.setup_subscription_link = setup_subscription_link;
/**
* External entry point for configuring the structural subscription.
@@ -1366,6 +1507,6 @@
}; // setup
}, '0.1', {requires: [
- 'dom', 'node', 'lazr.anim', 'lazr.formoverlay', 'lazr.overlay',
- 'lazr.effects', 'lp.app.errors', 'lp.client', 'gallery-accordion'
- ]});
+ 'dom', 'node', 'test', 'lazr.anim', 'lazr.formoverlay', 'lazr.overlay',
+ 'lazr.effects', 'lp.app.errors', 'lp.client', 'gallery-accordion'
+]});
Follow ups
