launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #03359
lp:~benji/launchpad/direct-personal-subscription-actions-scaffolding into lp:launchpad
Benji York has proposed merging lp:~benji/launchpad/direct-personal-subscription-actions-scaffolding into lp:launchpad with lp:~danilo/launchpad/bug728370-descriptions as a prerequisite.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~benji/launchpad/direct-personal-subscription-actions-scaffolding/+merge/58124
When viewing a bug's subscription page we want to offer the user actions
that they can take to reduce or eliminate the email they receive about
that particular bug.
Since there are several reasons a person can receive notifications about
a bug and the yellow team wants to collaborate on building the
functionality we decided to build up the scaffolding for the
functionality first so that team members can work in parallel on filling
it out. Since the functionality in question is hidden behind a feature
flag, we can land this incremental step. The scaffolding approach was
proposed by Gary on our pre-implementation call.
It works like this: the get_direct_subscription_information function
contains the pre-existing logic to determine what message should be
displayed about the current subscription situation and this branch adds
parallel logic to determine which subscription controls (mute,
subscribe, etc., encoded as element IDs) should be made visible to the
user.
The tests for the new functionality can be run by loading
lib/lp/bugs/javascript/tests/test_subscription.html in a browser.
To QA the changes you'll need to add these rules to /+feature-rules
(using tabs instead of spaces):
malone.advanced-structural-subscriptions.enabled default 1 on
malone.advanced-subscriptions.enabled default 2 on
Because the JS linter was broken for a while there is a large amount of
lint in this branch. That lint will be fixed in a follow-on branch
which will arrive shortly.
--
https://code.launchpad.net/~benji/launchpad/direct-personal-subscription-actions-scaffolding/+merge/58124
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~benji/launchpad/direct-personal-subscription-actions-scaffolding into lp:launchpad.
=== modified file 'lib/lp/bugs/browser/bugsubscription.py'
--- lib/lp/bugs/browser/bugsubscription.py 2011-04-07 21:31:29 +0000
+++ lib/lp/bugs/browser/bugsubscription.py 2011-04-18 13:40:20 +0000
@@ -595,7 +595,6 @@
cache = IJSONRequestCache(self.request).objects
cache.update(references)
cache['bug_subscription_info'] = subdata
-
@property
def label(self):
=== modified file 'lib/lp/bugs/javascript/subscription.js'
--- lib/lp/bugs/javascript/subscription.js 2011-04-14 20:03:32 +0000
+++ lib/lp/bugs/javascript/subscription.js 2011-04-18 13:40:20 +0000
@@ -449,17 +449,35 @@
}
+// 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: '#add-direct-subscription',
+ unsubscribe: '#remove-direct-subscription',
+ unsubscribe_with_warning: '#remove-direct-subscription-with-warning',
+ change_subscription: '#change-direct-subscription'
+}
+
/**
* Get direct subscription information.
*/
function get_direct_subscription_information(info) {
var reason;
var reasons = namespace._reasons;
+ var applicable_actions = [];
if (info.count == 0) {
+ // The user has subscriptions at all.
reason = reasons.NOT_SUBSCRIBED;
+ applicable_actions.push(action_ids.subscribe);
} else if (info.muted) {
+ // The user has a muted direct subscription.
reason = reasons.MUTED_SUBSCRIPTION;
+ applicable_actions.push(action_ids.unmute);
+ applicable_actions.push(action_ids.subscribe);
} 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 ' +
@@ -479,12 +497,25 @@
} else {
reason = reasons.YOU_SUBSCRIBED;
}
+ applicable_actions.push(action_ids.mute);
+ applicable_actions.push(action_ids.change_subscription);
+ if (info.count > 1) {
+ // The user has a non-personal subscription as well as a direct
+ // personal subscription.
+ applicable_actions.push(action_ids.unsubscribe_with_warning);
+ } else {
+ // The user just has the direct personal subscription.
+ applicable_actions.push(action_ids.unsubscribe);
+ }
+
} else {
// No direct subscriptions, but there are other
// subscriptions (because info.count != 0).
reason = reasons.NOT_PERSONALLY_SUBSCRIBED;
+ applicable_actions.push(action_ids.mute);
+ applicable_actions.push(action_ids.subscribe);
}
- return reason;
+ return {reason: reason, actions: applicable_actions};
}
namespace._get_direct_subscription_information =
get_direct_subscription_information;
@@ -563,6 +594,43 @@
}
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('mute-direct-subscription')
+ .set('text', 'MUTE'); // TODO fill in actual contents
+}
+
+function unmute_action() {
+ return make_action('unmute-direct-subscription')
+ .set('text', 'UNMUTE'); // TODO fill in actual contents
+}
+
+function subscribe_action() {
+ return make_action('add-direct-subscription')
+ .set('text', 'SUBSCRIBE'); // TODO fill in actual contents
+}
+
+function unsubscribe_action() {
+ return make_action('remove-direct-subscription')
+ .set('text', 'UNSUBSCRIBE'); // TODO fill in actual contents
+}
+
+function unsubscribe_with_warning_action() {
+ return make_action('remove-direct-subscription-with-warning')
+ .set('text', 'UNSUBSCRIBE WITH WARNING'); // TODO fill in actual contents
+}
+
+function change_subscription_action() {
+ return make_action('change-direct-subscription')
+ .set('text', 'CHANGE SUBSCRIPTION'); // TODO fill in actual contents
+}
+
/**
* Creates a node to store the direct subscription information.
*
@@ -572,10 +640,43 @@
* 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 direct_info = get_direct_subscription_information(info);
+ var direct_node = Y.Node.create('<div/>')
.set('id', 'direct-subscription')
- .set('text', direct);
+ .set('text', direct_info.reason)
+ // Make border box
+ .append(Y.Node.create('<div/>')
+ .addClass('actions')
+ .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', 'If you don\'t want to receive emails about '+
+ 'this bug you can:'))
+ .append(Y.Node.create('<div/>')
+ .setStyle('clear', 'both')
+ .setStyle('padding', '1em 0 0 1em')
+ .append(mute_action())
+ .append(unmute_action())
+ .append(subscribe_action())
+ .append(unsubscribe_action())
+ .append(change_subscription_action())))
+ // Add the unsubscribe action for when they have other subscriptions.
+ .append(unsubscribe_with_warning_action());
+
+ if (direct_info.actions.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('.actions').removeClass('hidden');
+ var i;
+ for (i=0; i<direct_info.actions.length; i++) {
+ direct_node.one(direct_info.actions[i]).removeClass('hidden');
+ }
+ }
return direct_node;
}
namespace._get_direct_description_node = get_direct_description_node;
=== modified file 'lib/lp/bugs/javascript/tests/test_subscription.js'
--- lib/lp/bugs/javascript/tests/test_subscription.js 2011-04-14 20:03:32 +0000
+++ lib/lp/bugs/javascript/tests/test_subscription.js 2011-04-18 13:40:20 +0000
@@ -887,7 +887,7 @@
* 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: {
@@ -908,7 +908,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(),
@@ -916,9 +916,13 @@
};
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(
+ ['#add-direct-subscription'],
+ direct_info.actions);
},
test_no_direct_subscriptions: function() {
@@ -929,9 +933,13 @@
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', '#add-direct-subscription'],
+ direct_info.actions);
},
test_muted_subscription: function() {
@@ -941,9 +949,13 @@
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));
+ direct_info.reason);
+ Y.ArrayAssert.itemsAreEqual(
+ ['#unmute-direct-subscription', '#add-direct-subscription'],
+ direct_info.actions);
},
test_direct_subscription: function() {
@@ -960,9 +972,14 @@
count: 1
};
+ var direct_info = module._get_direct_subscription_information(info);
Y.Assert.areSame(
module._reasons.YOU_SUBSCRIBED,
- module._get_direct_subscription_information(info));
+ direct_info.reason);
+ Y.ArrayAssert.itemsAreEqual(
+ ['#mute-direct-subscription', '#change-direct-subscription',
+ '#remove-direct-subscription'],
+ direct_info.actions);
},
test_direct_subscription_as_reporter: function() {
@@ -975,9 +992,15 @@
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', '#change-direct-subscription',
+ '#remove-direct-subscription'],
+ direct_info.actions);
},
test_direct_subscription_for_supervisor: function() {
@@ -992,9 +1015,14 @@
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', '#change-direct-subscription',
+ '#remove-direct-subscription'],
+ direct_info.actions);
},
test_direct_subscription_for_security_contact: function() {
@@ -1008,10 +1036,41 @@
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', '#change-direct-subscription',
+ '#remove-direct-subscription'],
+ direct_info.actions);
+ },
+
+ test_direct_subscription_and_other_subscriptions: function() {
+ // The simple direct subscription.
+ var sub = {
+ bug: {
+ 'private': false,
+ security_related: false,
+ },
+ principal_is_reporter: false,
+ };
+ 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', '#change-direct-subscription',
+ '#remove-direct-subscription-with-warning'],
+ direct_info.actions);
+ },
+
}));
@@ -1210,12 +1269,11 @@
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() {
@@ -1224,12 +1282,12 @@
direct: _constructCategory([{ bug: {} }]),
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);
},
}));
