launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #03379
[Merge] lp:~benji/launchpad/lint into lp:launchpad
Benji York has proposed merging lp:~benji/launchpad/lint into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~benji/launchpad/lint/+merge/58369
Fix lots of lint in subscriptions-related JS.
Tests that I ran manually (all of which passed):
lib/lp/registry/javascript/tests/test_distroseriesdifferences_details.html
lib/lp/registry/javascript/tests/test_distroseries.html
lib/lp/registry/javascript/tests/test_milestone_table.html
lib/lp/registry/javascript/tests/test_structural_subscription.html
lib/lp/registry/javascript/tests/test_timeline.html
lib/lp/registry/javascript/tests/timeline-iframe.html
lib/lp/bugs/javascript/tests/test_me_too.html
lib/lp/bugs/javascript/tests/test_subscriber.html
lib/lp/bugs/javascript/tests/test_subscription.html
--
https://code.launchpad.net/~benji/launchpad/lint/+merge/58369
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~benji/launchpad/lint into lp:launchpad.
=== modified file 'lib/lp/app/javascript/ajax_log.js'
--- lib/lp/app/javascript/ajax_log.js 2011-03-31 12:51:30 +0000
+++ lib/lp/app/javascript/ajax_log.js 2011-04-19 18:16:32 +0000
@@ -67,7 +67,7 @@
}).run();
/* Signify a new entry has been added to the log.
*/
- if (ajax_menu_animating == false) {
+ if (ajax_menu_animating === false) {
flash_menu.run();
ajax_menu_animating = true;
}
=== modified file 'lib/lp/app/javascript/calendar.js'
--- lib/lp/app/javascript/calendar.js 2010-07-15 11:06:14 +0000
+++ lib/lp/app/javascript/calendar.js 2011-04-19 18:16:32 +0000
@@ -23,7 +23,7 @@
*/
var pad_with_zero = function(num) {
num_as_string = String(num);
- if (num_as_string.length == 1) {
+ if (num_as_string.length === 1) {
num_as_string = "0" + num_as_string;
}
return num_as_string;
=== modified file 'lib/lp/app/javascript/choice.js'
--- lib/lp/app/javascript/choice.js 2011-03-07 01:44:54 +0000
+++ lib/lp/app/javascript/choice.js 2011-04-19 18:16:32 +0000
@@ -51,7 +51,7 @@
var cb = widget.get('contentBox');
var value = widget.get('value');
Y.Array.each(config.items, function(item) {
- if (item.value == value) {
+ if (item.value === value) {
cb.one('span').addClass(item.css_class);
} else {
cb.one('span').removeClass(item.css_class);
@@ -59,6 +59,6 @@
});
});
widget.render();
-}
+};
}, "0.1", {"requires": ["lazr.choiceedit", "lp.client.plugins"]});
=== modified file 'lib/lp/app/javascript/client.js'
--- lib/lp/app/javascript/client.js 2011-04-13 06:03:33 +0000
+++ lib/lp/app/javascript/client.js 2011-04-19 18:16:32 +0000
@@ -27,10 +27,10 @@
module.log_object = function(o, name) {
var result;
var format = function(value) {
- if (typeof value == 'string') {
+ if (typeof value === 'string') {
value = value.substring(0, 200); // Truncate long strings.
return '"' + value + '"';
- } else if (typeof value == 'function') {
+ } else if (typeof value === 'function') {
// Only log the function parameters instead
// of the whole code block.
return String(value).split(" {")[0];
@@ -45,13 +45,14 @@
var items = [];
var keys = [];
var key;
+ var index;
for (key in collection) {
if (collection.hasOwnProperty(key)) {
keys.push(key);
}
}
keys.sort();
- for (var index in keys) {
+ for (index in keys) {
if (keys.hasOwnProperty(index)) {
key = keys[index];
var value;
@@ -68,7 +69,7 @@
return items.join(',\n ');
};
- if (o === null || typeof o == 'string' || typeof o == 'function') {
+ if (o === null || typeof o === 'string' || typeof o === 'function') {
result = format(o);
} else {
result = '(direct-attributes)\n ' + introspect(o);
@@ -102,7 +103,7 @@
Does nothing to a relative URI that includes the root.*/
var host_start = uri.indexOf('//');
- if (host_start != -1) {
+ if (host_start !== -1) {
var host_end = uri.indexOf('/', host_start+2);
// eg. "http://www.example.com/api/devel/foo";;
// Don't try to insert the service base into what was an
@@ -152,7 +153,7 @@
module.get_field_uri = function(base_uri, field_name) {
base_uri = module.normalize_uri(base_uri);
field_name = escape(field_name);
- if (base_uri.charAt(base_uri.length - 1) == '/') {
+ if (base_uri.charAt(base_uri.length - 1) === '/') {
return base_uri + field_name;
} else {
return base_uri + '/' + field_name;
@@ -181,59 +182,73 @@
function update_cached_object(cache_name, cache, entry)
{
- var fields_changed = new Array();
- for (var name in cache) {
- var old_value = cache[name];
- var new_value = entry.get(name);
- if (name != 'lp_html') {
- if (old_value != new_value) {
- fields_changed.push(name);
- cache[name] = new_value;
- var field_updated_event_name =
- 'lp:' + cache_name + ':' + name + ':changed';
- var new_value_html = entry.getHTML(name);
- var event = {
- 'name': name,
- 'old_value': old_value,
- 'new_value': new_value,
- 'new_value_html': new_value_html,
- 'entry': entry
- };
- Y.fire(field_updated_event_name, event);
+ var fields_changed = [];
+ var name;
+ var html_name;
+ for (name in cache) {
+ if (cache.hasOwnProperty(name)) {
+ var old_value = cache[name];
+ var new_value = entry.get(name);
+ if (name !== 'lp_html') {
+ if (old_value !== new_value) {
+ fields_changed.push(name);
+ cache[name] = new_value;
+ var field_updated_event_name =
+ 'lp:' + cache_name + ':' + name + ':changed';
+ var new_value_html = entry.getHTML(name);
+ var event = {
+ 'name': name,
+ 'old_value': old_value,
+ 'new_value': new_value,
+ 'new_value_html': new_value_html,
+ 'entry': entry
+ };
+ Y.fire(field_updated_event_name, event);
+ }
}
- }
- else {
- // Since we don't care here about the content, we aren't using the
- // values here to determine if the field has changed, so we can just
- // update the cache.
- for (var html_name in old_value) {
- old_value[html_name] = new_value[html_name];
+ else {
+ // Since we don't care here about the content, we aren't using the
+ // values here to determine if the field has changed, so we can just
+ // update the cache.
+ for (html_name in old_value) {
+ if (old_value.hasOwnProperty(html_name)) {
+ old_value[html_name] = new_value[html_name];
+ }
+ }
}
}
}
if (fields_changed.length > 0) {
var event_name = 'lp:' + cache_name + ':changed';
- var event = {
+ var event_ = {
'fields_changed': fields_changed,
'entry': entry
};
- Y.fire(event_name, event);
+ Y.fire(event_name, event_);
}
}
module.update_cache = function(entry) {
- if (!entry) return;
+ if (!entry) {
+ return;
+ }
var original_uri = entry.lp_original_uri;
var full_uri = module.get_absolute_uri(original_uri);
- for (var name in LP.cache) {
- var cached_object = LP.cache[name];
- if (!Y.Lang.isValue(cached_object))
+ var name;
+ var cached_object;
+ for (name in LP.cache) {
+ if (LP.cache.hasOwnProperty(name)) {
+ cached_object = LP.cache[name];
+ /*jslint continue:true*/
+ if (!Y.Lang.isValue(cached_object)) {
continue;
- if (cached_object['self_link'] == full_uri) {
- Y.log(name + ' cached object has been updated.');
- update_cached_object(name, cached_object, entry);
+ }
+ if (cached_object.self_link === full_uri) {
+ Y.log(name + ' cached object has been updated.');
+ update_cached_object(name, cached_object, entry);
+ }
}
}
};
@@ -246,7 +261,7 @@
var representation, wrapped;
if (old_on_success) {
var media_type = response.getResponseHeader('Content-Type');
- if (media_type.substring(0,16) == 'application/json') {
+ if (media_type.substring(0,16) === 'application/json') {
representation = Y.JSON.parse(response.responseText);
display_notifications(
response.getResponseHeader('X-Lazr-Notifications'));
@@ -286,21 +301,22 @@
* @param notifications An json encoded array of (level, message) tuples.
*/
function display_notifications(notifications) {
- if (notifications === undefined)
+ if (notifications === undefined) {
return;
+ }
var notifications_by_level = {
'level10': {
- 'notifications': new Array()
+ 'notifications': []
},
'level20': {
- 'notifications': new Array()
+ 'notifications': []
},
'level30': {
- 'notifications': new Array()
+ 'notifications': []
},
'level40': {
- 'notifications': new Array()
+ 'notifications': []
}
};
@@ -320,7 +336,7 @@
var css_class = NOTIFICATION_INFO[key].css_class;
var node = Y.Node.create("<div class='"+css_class+"'/>");
node.set('innerHTML', notification);
- if (last_message==null) {
+ if (last_message === null) {
var div = Y.one('div#request-notifications');
div.insert(node);
} else {
@@ -395,6 +411,7 @@
/* Initialize a resource with its representation and URI. */
this.lp_client = client;
this.lp_original_uri = uri;
+ var key;
for (key in representation) {
if (representation.hasOwnProperty(key)) {
this[key] = representation[key];
@@ -427,8 +444,8 @@
on.failure = function(ignore, response, args) {
var client = args[0];
var original_url = args[1];
- if (response.status == module.HTTP_NOT_FOUND ||
- response.status == module.HTTP_SEE_ALSO) {
+ if (response.status === module.HTTP_NOT_FOUND ||
+ response.status === module.HTTP_SEE_ALSO) {
var file = new HostedFile(client, original_url);
return on_success(file);
} else if (old_on_failure !== undefined) {
@@ -495,6 +512,7 @@
// Copy the representation keys into our own set of attributes, and add
// an attribute-change event listener for caching purposes.
+ var key;
for (key in representation) {
if (representation.hasOwnProperty(key)) {
this.addAttr(key, {value: representation[key]});
@@ -510,7 +528,7 @@
Entry.prototype.mark_as_dirty = function(event) {
/* Respond to an event triggered by modification to an Entry's field. */
- if (event.newVal != event.prevVal) {
+ if (event.newVal !== event.prevVal) {
this.dirty_attributes.push(event.attrName);
}
};
@@ -596,6 +614,7 @@
/* Retrieve the value of a named GET operation on the given URI. */
var parameters = config.parameters;
var data = module.append_qs("", "ws.op", operation_name);
+ var name;
for (name in parameters) {
if (parameters.hasOwnProperty(name)) {
data = module.append_qs(data, name, parameters[name]);
@@ -609,8 +628,10 @@
/* Perform a named POST operation on the given URI. */
var on = Y.merge(config.on);
var parameters = config.parameters;
+ var data;
+ var name;
uri = module.normalize_uri(uri);
- var data = module.append_qs(data, "ws.op", operation_name);
+ data = module.append_qs(data, "ws.op", operation_name);
for (name in parameters) {
if (parameters.hasOwnProperty(name)) {
data = module.append_qs(data, name, parameters[name]);
@@ -620,7 +641,7 @@
var old_on_success = on.success;
on.success = function(unknown, response, args) {
- if (response.status == module.HTTP_CREATED) {
+ if (response.status === module.HTTP_CREATED) {
// A new object was created as a result of the operation.
// Get that object and run the callback on it instead.
var new_location = response.getResponseHeader("Location");
@@ -656,6 +677,7 @@
"Content-Type": "application/json",
"X-Content-Type-Override": "application/json"
};
+ var name;
if (headers !== undefined) {
for (name in headers) {
if (headers.hasOwnProperty(name)) {
@@ -758,7 +780,7 @@
return function(ioId, o) {
self.clearProgressUI();
// If it was a timeout...
- if (o.status == 503) {
+ if (o.status === 503) {
self.showError(
'Timeout error, please try again in a few minutes.');
// If it was a server error...
@@ -771,7 +793,7 @@
}
self.showError(server_error);
// Otherwise we send some sane text as an error
- } else if (o.status == 412){
+ } else if (o.status === 412){
self.showError(o.status + ' ' + o.statusText);
} else {
self.showError(self.get_generic_error(o));
@@ -802,7 +824,7 @@
return null;
}
return result[1];
-}
+};
FormErrorHandler.prototype.get_generic_error = function(response) {
@@ -812,7 +834,7 @@
else {
return response.status + ' ' + response.statusText;
}
-}
+};
module.FormErrorHandler = FormErrorHandler;
@@ -940,7 +962,7 @@
// Save the config object that the user passed in so that we can pass
// any extra parameters through to the lp.client constructor.
this.extra_config = config || {};
- this.extra_config['accept'] = 'application/json;include=lp_html';
+ this.extra_config.accept = 'application/json;include=lp_html';
// Save a reference to the original _saveData()
//method before wrapping it.
=== modified file 'lib/lp/bugs/javascript/bug_subscription.js'
--- lib/lp/bugs/javascript/bug_subscription.js 2011-03-23 16:28:51 +0000
+++ lib/lp/bugs/javascript/bug_subscription.js 2011-04-19 18:16:32 +0000
@@ -45,7 +45,7 @@
slide_in_anim.run();
Y.each(subscription_radio_buttons, function(subscription_button) {
subscription_button.on('click', function(e) {
- if(e.target.get('value') == 'update-subscription') {
+ if(e.target.get('value') === 'update-subscription') {
slide_out_anim.run();
} else {
slide_in_anim.run();
@@ -55,5 +55,5 @@
}
};
-}, "0.1", {"requires": ["dom", "node", "io-base", "lazr.anim", "lazr.effects",
+}, "0.1", {"requires": ["dom", "node", "io-base", "lazr.anim", "lazr.effects"
]});
=== modified file 'lib/lp/bugs/javascript/bugtask_index.js'
--- lib/lp/bugs/javascript/bugtask_index.js 2011-04-12 12:16:03 +0000
+++ lib/lp/bugs/javascript/bugtask_index.js 2011-04-19 18:16:32 +0000
@@ -265,7 +265,7 @@
},
failure: function(id, request) {
dupe_span.removeClass('update-in-progress-message');
- if (request.status == 400) {
+ if (request.status === 400) {
duplicate_form_overlay.showError(
new_dup_id + ' is not a valid bug number or' +
' nickname.');
@@ -384,9 +384,12 @@
display_privacy_notification();
} else {
if (bugs_private_notification_enabled) {
- if (Y.one('.global-notification').hasClass('hidden')) {
- Y.one('.portlet.private').setStyle('color', '#333');
- Y.one('.portlet.private').setStyle('background-color', '#fbfbfb');
+ if (Y.one('.global-notification').hasClass(
+ 'hidden')) {
+ Y.one('.portlet.private').setStyles({
+ color: '#333',
+ backgroundColor: '#fbfbfb'
+ });
}
}
Y.one('body').replaceClass('private', 'public');
@@ -440,9 +443,10 @@
this is because we have no way to use feature flags in
css directly. This should be remove if the feature
is accepted. */
- Y.one('body').addClass('feature-flag-bugs-private-notification-enabled');
+ var body = Y.one('body');
+ body.addClass('feature-flag-bugs-private-notification-enabled');
/* Set the visible flag so that the content moves down. */
- Y.one('body').addClass('global-notification-visible');
+ body.addClass('global-notification-visible');
if (Y.one('.global-notification').hasClass('hidden')) {
Y.one('.global-notification').addClass('transparent');
@@ -722,7 +726,7 @@
status_choice_edit.on('save', function(e) {
var cb = status_choice_edit.get('contentBox');
Y.Array.each(conf.status_widget_items, function(item) {
- if (item.value == status_choice_edit.get('value')) {
+ if (item.value === status_choice_edit.get('value')) {
cb.addClass(item.css_class);
} else {
cb.removeClass(item.css_class);
@@ -755,7 +759,7 @@
importance_choice_edit.on('save', function(e) {
var cb = importance_choice_edit.get('contentBox');
Y.Array.each(conf.importance_widget_items, function(item) {
- if (item.value == importance_choice_edit.get('value')) {
+ if (item.value === importance_choice_edit.get('value')) {
cb.addClass(item.css_class);
} else {
cb.removeClass(item.css_class);
@@ -811,9 +815,9 @@
inline_combo.set('value', null);
Y.Array.each(
milestone_choice_edit.get('items'), function(item) {
- if (item.value == milestone_choice_edit.get('value')) {
+ if (item.value === milestone_choice_edit.get('value')) {
inline_combo.all('option').each(function(opt) {
- if (opt.get('innerHTML') == item.name) {
+ if (opt.get('innerHTML') === item.name) {
opt.set('selected', true);
}
});
@@ -853,7 +857,8 @@
var yesno_content_template =
"<p>{person_name} did not previously have any " +
"assigned bugs in {pillar}.</p>" +
- "<p>Do you really want to assign them to this bug?</p>";
+ "<p>Do you really want to assign them to this bug?"+
+ "</p>";
var yesno_content = Y.Lang.substitute(
yesno_content_template,
{person_name: person, pillar: pillar});
@@ -881,7 +886,7 @@
};
var step_title =
- (conf.assignee_vocabulary == 'ValidAssignee') ?
+ (conf.assignee_vocabulary === 'ValidAssignee') ?
"Search for people or teams" :
"Select a team of which you are a member";
var assignee_picker = Y.lp.app.picker.addPickerPatcher(
@@ -1003,7 +1008,7 @@
var others_affected_count = this.get('others_affected_count');
var source_names = this._getSourceNames(others_affected_count);
Y.each(this.get('items'), function(item) {
- if (item.value in source_names) {
+ if (source_names.hasOwnProperty(item.value)) {
item.source_name = source_names[item.value];
}
});
@@ -1017,7 +1022,7 @@
_getSourceNames: function(others_affected_count) {
var source_names = {};
// What to say when the user is marked as affected.
- if (others_affected_count == 1) {
+ if (others_affected_count === 1) {
source_names[true] = (
'This bug affects you and 1 other person');
}
@@ -1030,7 +1035,7 @@
source_names[true] = 'This bug affects you';
}
// What to say when the user is marked as not affected.
- if (others_affected_count == 1) {
+ if (others_affected_count === 1) {
source_names[false] = (
'This bug affects 1 person, but not you');
}
=== modified file 'lib/lp/bugs/javascript/subscriber.js'
--- lib/lp/bugs/javascript/subscriber.js 2011-02-24 00:23:04 +0000
+++ lib/lp/bugs/javascript/subscriber.js 2011-04-19 18:16:32 +0000
@@ -90,7 +90,7 @@
Y.all('#subscribers-links div').each(function(link) {
var name = link.one('a').getAttribute('name');
- if (name == display_name) {
+ if (name === display_name) {
already_subscribed = true;
}
});
@@ -117,7 +117,7 @@
*/
'is_current_user_subscribing': function() {
return (
- this.get('subscriber').get('name') ==
+ this.get('subscriber').get('name') ===
this.get('person').get('name')
);
},
@@ -174,11 +174,11 @@
if (Y.Lang.isValue(text)) {
var link = this.get('link');
link.set('innerHTML', text);
- if (text == namespace.subscription_labels.SUBSCRIBE) {
+ if (text === namespace.subscription_labels.SUBSCRIBE) {
link.removeClass('modify remove');
link.removeClass('modify edit');
link.addClass('add');
- } else if (text == namespace.subscription_labels.EDIT) {
+ } else if (text === namespace.subscription_labels.EDIT) {
link.removeClass('add');
link.addClass('modify edit');
} else {
=== modified file 'lib/lp/bugs/javascript/subscription.js'
--- lib/lp/bugs/javascript/subscription.js 2011-04-18 16:51:04 +0000
+++ lib/lp/bugs/javascript/subscription.js 2011-04-19 18:16:32 +0000
@@ -684,20 +684,46 @@
}
+// This mapping contains the IDs of elements that will be made visible if they
+// apply to the current bug.
+var action_ids = {
+ mute: 'mute-direct-subscription',
+ unmute: 'unmute-direct-subscription',
+ subscribe_all: 'select-direct-subscription-discussion',
+ subscribe_metadata: 'select-direct-subscription-metadata',
+ subscribe_closed: 'select-direct-subscription-lifecycle',
+ subscribe_only_metadata: 'select-only-direct-subscription-metadata',
+ subscribe_only_closed: 'select-only-direct-subscription-lifecycle',
+ unsubscribe: 'remove-direct-subscription',
+ unsubscribe_with_warning: 'remove-direct-subscription-with-warning'
+};
+
/**
* Get direct subscription information.
*/
function get_direct_subscription_information(info) {
var reason;
var reasons = namespace._reasons;
+ var reductions = [];
+ var increases = [];
if (info.count === 0) {
+ // The user has no subscriptions at all.
reason = reasons.NOT_SUBSCRIBED;
+ increases.push(action_ids.subscribe_all);
+ increases.push(action_ids.subscribe_metadata);
+ increases.push(action_ids.subscribe_closed);
} else if (info.muted) {
+ // The user has a muted direct subscription.
reason = reasons.MUTED_SUBSCRIPTION;
+ increases.push(action_ids.unmute);
+ increases.push(action_ids.subscribe_all);
+ increases.push(action_ids.subscribe_metadata);
+ increases.push(action_ids.subscribe_closed);
} else if (info.direct.personal.length > 0) {
+ // The user has a direct personal subscription.
if (info.direct.personal.length > 1) {
Y.error(
- 'A person should not have more than ' +
+ 'Programmer error: a person should not have more than ' +
'one direct personal subscription.');
}
var subscription = info.direct.personal[0];
@@ -714,12 +740,42 @@
} else {
reason = reasons.YOU_SUBSCRIBED;
}
+ reductions.push(action_ids.mute);
+ switch (subscription.subscription.bug_notification_level) {
+ case 'Discussion':
+ reductions.push(action_ids.subscribe_only_metadata);
+ reductions.push(action_ids.subscribe_only_closed);
+ break;
+ case 'Details':
+ increases.push(action_ids.subscribe_all);
+ reductions.push(action_ids.subscribe_only_closed);
+ break;
+ case 'Lifecycle':
+ increases.push(action_ids.subscribe_all);
+ increases.push(action_ids.subscribe_metadata);
+ break;
+ default:
+ Y.error('Programmer error: unknown bug notification level: '+
+ subscription.subscription.bug_notification_level);
+ }
+ if (info.count > 1) {
+ // The user has a non-personal subscription as well as a direct
+ // personal subscription.
+ reductions.push(action_ids.unsubscribe_with_warning);
+ } else {
+ // The user just has the direct personal subscription.
+ reductions.push(action_ids.unsubscribe);
+ }
+
} else {
// No direct subscriptions, but there are other
// subscriptions (because info.count != 0).
reason = reasons.NOT_PERSONALLY_SUBSCRIBED;
+ reductions.push(action_ids.mute);
+ reductions.push(action_ids.subscribe_only_metadata);
+ reductions.push(action_ids.subscribe_only_closed);
}
- return reason;
+ return {reason: reason, reductions: reductions, increases: increases};
}
namespace._get_direct_subscription_information =
get_direct_subscription_information;
@@ -804,6 +860,73 @@
}
namespace._safely_render_description = safely_render_description;
+function make_action(id) {
+ return Y.Node.create('<div/>')
+ .addClass('hidden')
+ .set('id', id);
+}
+
+function mute_action() {
+ return make_action(action_ids.mute)
+ .set('text', 'MUTE'); // TODO fill in actual contents
+}
+
+function unmute_action() {
+ return make_action(action_ids.unmute)
+ .set('text', 'UNMUTE'); // TODO fill in actual contents
+}
+
+function subscribe_all_action() {
+ return make_action(action_ids.subscribe_all)
+ .set('text', 'SUBSCRIBE ALL'); // TODO fill in actual contents
+}
+
+function subscribe_metadata_action() {
+ return make_action(action_ids.subscribe_metadata)
+ .set('text', 'SUBSCRIBE METADATA'); // TODO fill in actual contents
+}
+
+function subscribe_closed_action() {
+ return make_action(action_ids.subscribe_closed)
+ .set('text', 'SUBSCRIBE CLOSED'); // TODO fill in actual contents
+}
+
+function subscribe_only_metadata_action() {
+ return make_action(action_ids.subscribe_only_metadata)
+ .set('text', 'SUBSCRIBE ONLY METADATA'); // TODO fill in
+}
+
+function subscribe_only_closed_action() {
+ return make_action(action_ids.subscribe_only_closed)
+ .set('text', 'SUBSCRIBE ONLY CLOSED'); // TODO fill in actual contents
+}
+
+function unsubscribe_action() {
+ return make_action(action_ids.unsubscribe)
+ .set('text', 'UNSUBSCRIBE'); // TODO fill in actual contents
+}
+
+function unsubscribe_with_warning_action() {
+ return make_action(action_ids.unsubscribe_with_warning)
+ .set('text', 'UNSUBSCRIBE WITH WARNING'); // TODO fill in
+}
+
+function border_box(title, content_div) {
+ return Y.Node.create('<div/>')
+ .addClass('hidden')
+ .setStyle('border', '1px solid #ddd')
+ .setStyle('padding', '0 1em 1em 1em')
+ .append(Y.Node.create('<span/>')
+ .setStyle('backgroundColor', '#fff')
+ .setStyle('float', 'left')
+ .setStyle('marginTop', '-0.6em')
+ .setStyle('padding', '0 1ex')
+ .set('text', title))
+ .append(content_div
+ .setStyle('clear', 'both')
+ .setStyle('padding', '1em 0 0 1em'));
+}
+
/**
* Creates a node to store the direct subscription information.
*
@@ -813,10 +936,51 @@
* personal subscriptions (if any).
*/
function get_direct_description_node(info) {
- var direct = get_direct_subscription_information(info);
- var direct_node = Y.Node.create('<div></div>')
+ var i;
+ var direct_info = get_direct_subscription_information(info);
+ var less_email_title = 'If you don\'t want to receive emails about '+
+ 'this bug you can';
+ var more_email_title = 'If you want to receive more emails about '+
+ 'this bug you can';
+ var direct_node = Y.Node.create('<div/>')
.set('id', 'direct-subscription')
- .set('text', direct);
+ .set('text', direct_info.reason)
+ // Make border box for actions that reduce email.
+ .append(border_box(less_email_title, Y.Node.create('<div/>')
+ .append(mute_action())
+ .append(subscribe_only_metadata_action())
+ .append(subscribe_only_closed_action())
+ .append(unsubscribe_action()))
+ .addClass('reductions'))
+ // Make border box for actions that increase email.
+ .append(border_box(more_email_title, Y.Node.create('<div/>')
+ .append(unmute_action())
+ .append(subscribe_all_action())
+ .append(subscribe_metadata_action())
+ .append(subscribe_closed_action()))
+ .addClass('increases'))
+ // Add the unsubscribe action for when they have other subscriptions.
+ .append(unsubscribe_with_warning_action());
+
+ if (direct_info.reductions.length !== 0) {
+ // If there are any actions the user can take, unhide the actions box
+ // and then unhide the specific actions that they should see.
+ direct_node.one('.reductions').removeClass('hidden');
+ for (i=0; i<direct_info.reductions.length; i++) {
+ direct_node.one('#'+direct_info.reductions[i])
+ .removeClass('hidden');
+ }
+ }
+
+ if (direct_info.increases.length !== 0) {
+ // If there are any actions the user can take, unhide the actions box
+ // and then unhide the specific actions that they should see.
+ direct_node.one('.increases').removeClass('hidden');
+ for (i=0; i<direct_info.increases.length; i++) {
+ direct_node.one('#'+direct_info.increases[i])
+ .removeClass('hidden');
+ }
+ }
return direct_node;
}
namespace._get_direct_description_node = get_direct_description_node;
=== modified file 'lib/lp/bugs/javascript/tests/test_subscriber.js'
--- lib/lp/bugs/javascript/tests/test_subscriber.js 2011-02-25 03:54:13 +0000
+++ lib/lp/bugs/javascript/tests/test_subscriber.js 2011-04-19 18:16:32 +0000
@@ -39,7 +39,7 @@
'The escaped user uri should be the same as the unescaped uri.');
Y.Assert.isNull(
this.subscriber.get('user_node'),
- 'User node should not be known and should be null at this point.');
+ 'User node should not be known and be null at this point.');
Y.Assert.areSame(
'',
this.subscriber.get('css_name'),
@@ -172,9 +172,9 @@
// LP is global.
window.LP = {
cache: {},
- links: {},
+ links: {}
};
- Y.lp.client.Launchpad = function() {}
+ Y.lp.client.Launchpad = function() {};
},
tearDown: function() {
=== modified file 'lib/lp/bugs/javascript/tests/test_subscription.js'
--- lib/lp/bugs/javascript/tests/test_subscription.js 2011-04-18 16:51:04 +0000
+++ lib/lp/bugs/javascript/tests/test_subscription.js 2011-04-19 18:16:32 +0000
@@ -259,7 +259,7 @@
personal: [],
as_team_member: [],
as_team_admin: [{ principal: 'team1'},
- { principal: 'team2'}]
+ { principal: 'team2'}]
};
var subs = module._gather_subscriptions_as_assignee(mock_category);
Y.Assert.areEqual(1, subs.length);
@@ -947,13 +947,16 @@
* Tests for method get_direct_subscription_information().
*/
suite.add(new Y.Test.Case({
- name: 'Get reason for a direct subscription',
+ name: 'Get reason and actions for a direct subscription',
_should: {
error: {
test_multiple_direct_subscriptions:
- new Error('A person should not have more than ' +
- 'one direct personal subscription.')
+ new Error('Programmer error: a person should not have more than '+
+ 'one direct personal subscription.'),
+ test_direct_subscription_at_unknown_level:
+ new Error('Programmer error: unknown bug notification level: '+
+ 'The Larch')
}
},
@@ -968,7 +971,7 @@
module._get_direct_subscription_information(info);
},
- test_no_subscriptions: function() {
+ test_no_subscriptions_at_all: function() {
// There are no subscriptions at all.
var info = {
direct: _constructCategory(),
@@ -976,9 +979,18 @@
};
info.count = info.direct.count + info.from_duplicates.count;
+ direct_info = module._get_direct_subscription_information(info);
Y.Assert.areEqual(
module._reasons.NOT_SUBSCRIBED,
- module._get_direct_subscription_information(info));
+ direct_info.reason);
+ Y.ArrayAssert.itemsAreEqual(
+ [],
+ direct_info.reductions);
+ Y.ArrayAssert.itemsAreEqual(
+ ['select-direct-subscription-discussion',
+ 'select-direct-subscription-metadata',
+ 'select-direct-subscription-lifecycle'],
+ direct_info.increases);
},
test_no_direct_subscriptions: function() {
@@ -989,9 +1001,18 @@
from_duplicates: _constructCategory(['dupe'])
};
info.count = info.direct.count + info.from_duplicates.count;
+ direct_info = module._get_direct_subscription_information(info);
Y.Assert.areSame(
module._reasons.NOT_PERSONALLY_SUBSCRIBED,
- module._get_direct_subscription_information(info));
+ direct_info.reason);
+ Y.ArrayAssert.itemsAreEqual(
+ ['mute-direct-subscription',
+ 'select-only-direct-subscription-metadata',
+ 'select-only-direct-subscription-lifecycle'],
+ direct_info.reductions);
+ Y.ArrayAssert.itemsAreEqual(
+ [],
+ direct_info.increases);
},
test_muted_subscription: function() {
@@ -1001,44 +1022,152 @@
muted: true
};
info.count = info.direct.count;
+ direct_info = module._get_direct_subscription_information(info);
Y.Assert.areSame(
module._reasons.MUTED_SUBSCRIPTION,
- module._get_direct_subscription_information(info));
- },
-
- test_direct_subscription: function() {
- // The simple direct subscription.
- var sub = {
- bug: {
- //private: false, // JSLint complains
- security_related: false
- },
- principal_is_reporter: false
- };
- sub.bug['private'] = false;
- var info = {
- direct: _constructCategory([sub]),
- count: 1
- };
-
- Y.Assert.areSame(
- module._reasons.YOU_SUBSCRIBED,
- module._get_direct_subscription_information(info));
+ direct_info.reason);
+ Y.ArrayAssert.itemsAreEqual(
+ [],
+ direct_info.reductions);
+ Y.ArrayAssert.itemsAreEqual(
+ ['unmute-direct-subscription',
+ 'select-direct-subscription-discussion',
+ 'select-direct-subscription-metadata',
+ 'select-direct-subscription-lifecycle'],
+ direct_info.increases);
+ },
+
+ test_direct_subscription_at_discussion_level: function() {
+ // The larch^D^D^D^D^D^D simple direct subscription.
+ var sub = {
+ bug: {
+ 'private': false,
+ security_related: false
+ },
+ principal_is_reporter: false,
+ subscription: {bug_notification_level: 'Discussion'}
+ };
+ var info = {
+ direct: _constructCategory([sub]),
+ count: 1
+ };
+
+ var direct_info = module._get_direct_subscription_information(info);
+ Y.Assert.areSame(
+ module._reasons.YOU_SUBSCRIBED,
+ direct_info.reason);
+ Y.ArrayAssert.itemsAreEqual(
+ ['mute-direct-subscription',
+ 'select-only-direct-subscription-metadata',
+ 'select-only-direct-subscription-lifecycle',
+ 'remove-direct-subscription'],
+ direct_info.reductions);
+ Y.ArrayAssert.itemsAreEqual(
+ [],
+ direct_info.increases);
+ },
+
+ test_direct_subscription_at_metadata_level: function() {
+ // The simple direct subscription at metadata level.
+ var sub = {
+ bug: {
+ 'private': false,
+ security_related: false
+ },
+ principal_is_reporter: false,
+ subscription: {bug_notification_level: 'Details'}
+ };
+ var info = {
+ direct: _constructCategory([sub]),
+ count: 1
+ };
+
+ var direct_info = module._get_direct_subscription_information(info);
+ Y.Assert.areSame(
+ module._reasons.YOU_SUBSCRIBED,
+ direct_info.reason);
+ Y.ArrayAssert.itemsAreEqual(
+ ['mute-direct-subscription',
+ 'select-only-direct-subscription-lifecycle',
+ 'remove-direct-subscription'],
+ direct_info.reductions);
+ Y.ArrayAssert.itemsAreEqual(
+ ['select-direct-subscription-discussion'],
+ direct_info.increases);
+ },
+
+ test_direct_subscription_at_lifecycle_level: function() {
+ // The simple direct subscription at lifecycle level.
+ var sub = {
+ bug: {
+ 'private': false,
+ security_related: false
+ },
+ principal_is_reporter: false,
+ subscription: {bug_notification_level: 'Lifecycle'}
+ };
+ var info = {
+ direct: _constructCategory([sub]),
+ count: 1
+ };
+
+ var direct_info = module._get_direct_subscription_information(info);
+ Y.Assert.areSame(
+ module._reasons.YOU_SUBSCRIBED,
+ direct_info.reason);
+ Y.ArrayAssert.itemsAreEqual(
+ ['mute-direct-subscription',
+ 'remove-direct-subscription'],
+ direct_info.reductions);
+ Y.ArrayAssert.itemsAreEqual(
+ ['select-direct-subscription-discussion',
+ 'select-direct-subscription-metadata'],
+ direct_info.increases);
+ },
+
+ test_direct_subscription_at_unknown_level: function() {
+ // The simple direct subscription at unknown level.
+ var sub = {
+ bug: {
+ 'private': false,
+ security_related: false
+ },
+ principal_is_reporter: false,
+ subscription: {bug_notification_level: 'The Larch'}
+ };
+ var info = {
+ direct: _constructCategory([sub]),
+ count: 1
+ };
+ // This should raise an error.
+ module._get_direct_subscription_information(info);
},
test_direct_subscription_as_reporter: function() {
// The direct subscription created for bug reporter.
var sub = {
bug: {},
- principal_is_reporter: true
+ principal_is_reporter: true,
+ subscription: {bug_notification_level: 'Discussion'}
};
var info = {
direct: _constructCategory([sub]),
count: 1
};
+
+ var direct_info = module._get_direct_subscription_information(info);
Y.Assert.areSame(
module._reasons.YOU_REPORTED,
- module._get_direct_subscription_information(info));
+ direct_info.reason);
+ Y.ArrayAssert.itemsAreEqual(
+ ['mute-direct-subscription',
+ 'select-only-direct-subscription-metadata',
+ 'select-only-direct-subscription-lifecycle',
+ 'remove-direct-subscription'],
+ direct_info.reductions);
+ Y.ArrayAssert.itemsAreEqual(
+ [],
+ direct_info.increases);
},
test_direct_subscription_for_supervisor: function() {
@@ -1046,17 +1175,27 @@
// the bug supervisor.
var sub = {
bug: {
- //private: true //JSLint complains
- }
+ 'private': true
+ },
+ subscription: {bug_notification_level: 'Discussion'}
};
- sub.bug['private'] = true;
var info = {
direct: _constructCategory([sub]),
count: 1
};
+ var direct_info = module._get_direct_subscription_information(info);
Y.Assert.areSame(
module._reasons.YOU_SUBSCRIBED_BUG_SUPERVISOR,
- module._get_direct_subscription_information(info));
+ direct_info.reason);
+ Y.ArrayAssert.itemsAreEqual(
+ ['mute-direct-subscription',
+ 'select-only-direct-subscription-metadata',
+ 'select-only-direct-subscription-lifecycle',
+ 'remove-direct-subscription'],
+ direct_info.reductions);
+ Y.ArrayAssert.itemsAreEqual(
+ [],
+ direct_info.increases);
},
test_direct_subscription_for_security_contact: function() {
@@ -1064,15 +1203,58 @@
var sub = {
bug: {
security_related: true
- }
+ },
+ subscription: {bug_notification_level: 'Discussion'}
};
var info = {
direct: _constructCategory([sub]),
count: 1
};
+ var direct_info = module._get_direct_subscription_information(info);
Y.Assert.areSame(
module._reasons.YOU_SUBSCRIBED_SECURITY_CONTACT,
- module._get_direct_subscription_information(info));
+ direct_info.reason);
+ Y.ArrayAssert.itemsAreEqual(
+ ['mute-direct-subscription',
+ 'select-only-direct-subscription-metadata',
+ 'select-only-direct-subscription-lifecycle',
+ 'remove-direct-subscription'],
+ direct_info.reductions);
+ Y.ArrayAssert.itemsAreEqual(
+ [],
+ direct_info.increases);
+ },
+
+ test_direct_subscription_and_other_subscriptions: function() {
+ // Other subscriptions are present along with the simple direct
+ // subscription.
+ var sub = {
+ bug: {
+ 'private': false,
+ security_related: false
+ },
+ principal_is_reporter: false,
+ subscription: {bug_notification_level: 'Discussion'}
+ };
+ var info = {
+ direct: _constructCategory([sub]),
+ from_duplicates: _constructCategory(['dupe']),
+ count: 2
+ };
+
+ var direct_info = module._get_direct_subscription_information(info);
+ Y.Assert.areSame(
+ module._reasons.YOU_SUBSCRIBED,
+ direct_info.reason);
+ Y.ArrayAssert.itemsAreEqual(
+ ['mute-direct-subscription',
+ 'select-only-direct-subscription-metadata',
+ 'select-only-direct-subscription-lifecycle',
+ 'remove-direct-subscription-with-warning'],
+ direct_info.reductions);
+ Y.ArrayAssert.itemsAreEqual(
+ [],
+ direct_info.increases);
}
}));
@@ -1272,26 +1454,27 @@
direct: _constructCategory(),
count: 0
};
- var expected_text = module._get_direct_subscription_information(info);
+ var expected_text = module._get_direct_subscription_information(
+ info).reason;
var node = module._get_direct_description_node(info);
- Y.Assert.areEqual(
- 'direct-subscription', node.get('id'));
- Y.Assert.areEqual(
- expected_text, node.get('text'));
+ Y.Assert.areEqual('direct-subscription', node.get('id'));
+ Y.Assert.isTrue(node.get('text').indexOf(expected_text) !== -1);
},
test_direct_subscription: function() {
// One personal, direct subscription exists.
var info = {
- direct: _constructCategory([{ bug: {} }]),
+ direct: _constructCategory([{
+ bug: {},
+ subscription: {bug_notification_level: 'Discussion'} }]),
count: 1
};
- var expected_text = module._get_direct_subscription_information(info);
+ var expected_text = module._get_direct_subscription_information(
+ info).reason;
var node = module._get_direct_description_node(info);
Y.Assert.areEqual(
'direct-subscription', node.get('id'));
- Y.Assert.areEqual(
- expected_text, node.get('text'));
+ Y.Assert.isTrue(node.get('text').indexOf(expected_text) !== -1);
}
}));
=== modified file 'lib/lp/code/javascript/branch.subscription.js'
--- lib/lp/code/javascript/branch.subscription.js 2011-02-24 00:23:04 +0000
+++ lib/lp/code/javascript/branch.subscription.js 2011-04-19 18:16:32 +0000
@@ -60,7 +60,8 @@
* @attribute direct_subscribers {Y.Node} Wrapper div for the subscriber list.
* @attribute self_subscribe {Y.Node} Container for "Subscribe" link.
*/
-var SubscriptionWidget = function(config) {
+var SubscriptionWidget;
+SubscriptionWidget = function(config) {
SubscriptionWidget.superclass.constructor.apply(this, arguments);
};
@@ -84,7 +85,8 @@
var form_url = this.get("self_subscribe").get("href") + '/++form++';
var lp_client = this._lp_client;
- var form_overlay = new Y.lazr.FormOverlay({
+ var form_overlay;
+ form_overlay = new Y.lazr.FormOverlay({
headerContent: "<h2>Subscribe to branch</h2>",
form_submit_button: Y.Node.create(
'<button type="submit" name="field.actions.subscribe" ' +
=== modified file 'lib/lp/registry/javascript/tests/test_structural_subscription.js'
--- lib/lp/registry/javascript/tests/test_structural_subscription.js 2011-04-19 01:17:31 +0000
+++ lib/lp/registry/javascript/tests/test_structural_subscription.js 2011-04-19 18:16:32 +0000
@@ -627,7 +627,7 @@
Assert.isTrue(submit_button.hasClass('spinner'));
Assert.isFalse(submit_button.hasClass('lazr-pos'));
// Now we resume the call to trigger the failure.
- this.configuration.lp_client.named_post.resume()
+ this.configuration.lp_client.named_post.resume();
// The spinner is gone.
Assert.isTrue(submit_button.hasClass('lazr-pos'));
Assert.isFalse(submit_button.hasClass('spinner'));
