launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #03305
[Merge] lp:~danilo/launchpad/bug728370-direct-subs into lp:launchpad
Данило Шеган has proposed merging lp:~danilo/launchpad/bug728370-direct-subs into lp:launchpad with lp:~danilo/launchpad/bug728370-nondirect-subs as a prerequisite.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~danilo/launchpad/bug728370-direct-subs/+merge/57464
= Bug 728370: show subscription descriptions =
This is part of work on bug 728370: displaying user-readable descriptions of all the subscriptions a person has either directly or implicitely to a particular bug. Main goal of the page is for people to be able to unsubscribe quickly from bug mail. This branch does not worry about structural subscriptions.
I'll fix lint issues after the review (so diff doesn't grow even further). Diff is slightly over-sized, but majority of the changes are tests (about 2/3 of the diff).
== Proposed fix ==
Continuing the work on the branch this depends on (getting landed), we introduce a direct subscription handler and tests for it. It directly returns the reason (unlike "other" subscriptions which also returned potential variable replacements).
Also, this includes actual Y.Node construction for in-page elements containing these descriptions, but it is ugly on-purpose. Prettyfying should be the next, independent step (branch). Since this is protected by the feature flag, no harm is done by landing this as-is.
In future steps (not part of 728370), we'll also introduce actions for each of these subscriptions. The goal is to get this landed and allow people to independently work on prettyfying and adding suitable actions for unsubscribing.
== Tests ==
lib/lp/bugs/javascript/tests/test_subscription.html
bin/test -cvvt PersonSubscriptionInfo.*getDataForClient
== Demo and Q/A ==
Look at:
https://bugs.launchpad.dev/firefox/+bug/1/+subscriptions
(note, add "malone.advanced-structural-subscriptions.enabled default 1 on" to launchpad.dev/+feature-rules)
To facilitate the demo, add a few subscriptions:
- directly subscribe to bug 1
- subscribe one of your teams you are a member of
- subscribe one of your teams you are an admin of
- make bugs 2 and 3 duplicates of bug 1
- subscribe personally/as-team-member/as-team-admin to one of those
duplicates
- make yourself/your team the assignee on one or several bug tasks
= Launchpad lint =
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/bugs/javascript/tests/test_subscription.html
lib/lp/bugs/javascript/subscription.js
lib/lp/bugs/javascript/tests/test_subscription.js
lib/lp/bugs/templates/bug-subscription-list.pt
lib/lp/bugs/model/personsubscriptioninfo.py
lib/lp/bugs/model/tests/test_personsubscriptioninfo.py
./lib/lp/bugs/model/personsubscriptioninfo.py
119: E501 line too long (80 characters)
259: E301 expected 1 blank line, found 0
269: E301 expected 1 blank line, found 0
276: E301 expected 1 blank line, found 0
307: E202 whitespace before ')'
315: E202 whitespace before ')'
119: Line exceeds 78 characters.
./lib/lp/bugs/model/tests/test_personsubscriptioninfo.py
12: 'searchbuilder' imported but unused
29: 'anonymous_logged_in' imported but unused
8: 'Store' imported but unused
16: 'BugTaskStatus' imported but unused
10: 'ProxyFactory' imported but unused
9: 'Unauthorized' imported but unused
16: 'BugTaskImportance' imported but unused
29: 'login_person' imported but unused
26: 'BugSubscriptionFilter' imported but unused
13: 'IStore' imported but unused
37: E302 expected 2 blank lines, found 1
--
https://code.launchpad.net/~danilo/launchpad/bug728370-direct-subs/+merge/57464
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~danilo/launchpad/bug728370-direct-subs into lp:launchpad.
=== modified file 'lib/lp/bugs/javascript/subscription.js'
--- lib/lp/bugs/javascript/subscription.js 2011-04-13 06:26:27 +0000
+++ lib/lp/bugs/javascript/subscription.js 2011-04-13 10:56:37 +0000
@@ -26,6 +26,9 @@
*/
var reasons = {
NOT_SUBSCRIBED: "You are not subscribed to this bug.",
+ NOT_PERSONALLY_SUBSCRIBED: (
+ "You are not directly subscribed to this bug, " +
+ "but you have other subscriptions."),
MUTED_SUBSCRIPTION: "You have muted all email from this bug."
};
namespace._reasons = reasons;
@@ -77,10 +80,10 @@
/* These are the duplicate bug variations. */
var _SUBSCRIBED_TO_DUPLICATE = (
'a direct subscriber to bug {duplicate_bug}, which is marked as a ' +
- 'duplicate of this bug, {bug_id}');
+ 'duplicate of this bug, {bug_id}.');
var _SUBSCRIBED_TO_DUPLICATES = (
'a direct subscriber to bugs {duplicate_bugs}, which are marked as ' +
- 'duplicates of this bug, {bug_id}');
+ 'duplicates of this bug, {bug_id}.');
/* These are the actual strings to use. */
Y.mix(reasons, {
YOU_SUBSCRIBED_TO_DUPLICATE: _BECAUSE_YOU_ARE + _SUBSCRIBED_TO_DUPLICATE,
@@ -189,7 +192,7 @@
if (typeof(bug) == 'string') {
return bug;
} else {
- return ObjectLink(bug, 'bug #' + bug.id.toString(), bug.web_link);
+ return ObjectLink(bug, '#' + bug.id.toString(), bug.web_link);
}
}
@@ -416,17 +419,217 @@
}
-function get_subscription_reason(config) {
+/**
+ * Get direct subscription information.
+ */
+function get_direct_subscription_information(info) {
+ var reason;
+ var reasons = namespace._reasons;
+ if (info.count == 0) {
+ reason = reasons.NOT_SUBSCRIBED;
+ } else if (info.muted) {
+ reason = reasons.MUTED_SUBSCRIPTION;
+ } else if (info.direct.personal.length > 0) {
+ if (info.direct.personal.length > 1) {
+ Y.error(
+ 'A person should not have more than ' +
+ 'one direct personal subscription.');
+ }
+ var subscription = info.direct.personal[0];
+ var bug = subscription.bug;
+ if (subscription.principal_is_reporter) {
+ reason = reasons.YOU_REPORTED;
+ } else if (bug.private) {
+ reason = reasons.YOU_SUBSCRIBED_BUG_SUPERVISOR;
+ } else if (bug.security_related) {
+ // If bug is both private and security-related, you'll
+ // only get the description talking about privacy.
+ // Not considered a big deal.
+ reason = reasons.YOU_SUBSCRIBED_SECURITY_CONTACT;
+ } else {
+ reason = reasons.YOU_SUBSCRIBED;
+ }
+ } else {
+ // No direct subscriptions, but there are other
+ // subscriptions (because info.count != 0).
+ reason = reasons.NOT_PERSONALLY_SUBSCRIBED;
+ }
+ return reason;
+}
+namespace._get_direct_subscription_information =
+ get_direct_subscription_information;
+
+/**
+ * Returns an anchor element HTML for an ObjectLink element.
+ * It safely encodes the `title` and `url` elements to avoid any XSS vectors.
+ *
+ * @method get_objectlink_html
+ * @param {Object} element ObjectLink element or a simple string.
+ * @returns {String} HTML for the A element representing passed in
+ * ObjectLink `element`. If `element` is a string, return it unmodified.
+ */
+function get_objectlink_html(element) {
+ if (Y.Lang.isString(element)) {
+ return element;
+ } else if (Y.Lang.isObject(element)) {
+ if (element.url === undefined && element.title == undefined) {
+ Y.error('Not a proper ObjectLink.');
+ }
+ var node = Y.Node.create('<div></div>');
+ node.appendChild(
+ Y.Node.create('<a></a>')
+ .set('href', element.url)
+ .set('text', element.title));
+ var text = node.get('innerHTML');
+ node.destroy(true);
+ return text;
+ }
+}
+namespace._get_objectlink_html = get_objectlink_html;
+
+/**
+ * Array sort function for objects sorting them by their `title` property.
+ */
+function sort_by_title(a, b) {
+ return ((a.title == b.title) ? 0 :
+ ((a.title > b.title) ? 1 : -1));
+}
+
+/**
+ * Renders the description in a safe manner escaping HTML as appropriate.
+ *
+ * @method safely_render_description
+ * @param {Object} subscription Object containing the string `reason` and
+ * object `vars` containing variables to be replaced in `reason`.
+ * @param {Object} additional_vars Objects containing additional, global
+ * variables to also be replaced if not overridden.
+ * @returns {String} `reason` with all {var} occurrences replaced with
+ * appropriate subscription.vars[var] values.
+ */
+function safely_render_description(subscription, additional_vars) {
+ function var_replacer(key, vars) {
+ if (vars !== undefined) {
+ if (Y.Lang.isArray(vars)) {
+ vars.sort(sort_by_title);
+ // We want a plural concatenation.
+ var final_element = vars.pop();
+ var text_elements = [];
+ for (var index in vars) {
+ text_elements.push(get_objectlink_html(vars[index]));
+ };
+ return (text_elements.join(', ') +
+ ' and ' + get_objectlink_html(final_element));
+ } else {
+ return get_objectlink_html(vars);
+ }
+ } else {
+ if (Y.Lang.isObject(additional_vars) &&
+ additional_vars.hasOwnProperty(key)) {
+ return get_objectlink_html(additional_vars[key]);
+ }
+ }
+ }
+ return Y.substitute(subscription.reason, subscription.vars, var_replacer);
+}
+namespace._safely_render_description = safely_render_description
+
+/**
+ * Creates a node to store the direct subscription information.
+ *
+ * @param {Object} info LP.cache.bug_subscription_info object.
+ * @returns {Object} Y.Node with the ID of 'direct-description' and
+ * text set to the actual textual description of the direct
+ * 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>')
+ .set('id', 'direct-subscription')
+ .set('text', direct);
+ return direct_node;
+}
+namespace._get_direct_description_node = get_direct_description_node;
+
+/**
+ * Creates a node to store single subscription description.
+ *
+ * @param {Object} subscription Object containing `reason` and `vars`
+ * to be substituted into `reason` with safely_render_description.
+ * @param {Object} extra_data Extra variables to substitute.
+ * @returns {Object} Y.Node with the class 'bug-subscription-description'
+ * and textual description in a separate node with
+ * class 'description-text'.
+ */
+function get_single_description_node(subscription, extra_data) {
+ var node = Y.Node.create('<div></div>')
+ .addClass('subscription-description');
+ node.appendChild(
+ Y.Node.create('<div></div>')
+ .addClass('description-text')
+ .set('innerHTML',
+ safely_render_description(subscription, extra_data)));
+ return node;
+}
+namespace._get_single_description_node = get_single_description_node;
+
+/**
+ * Creates a node to store "other" subscriptions information.
+ * "Other" means any bug subscriptions which are not personal and direct.
+ *
+ * @param {Object} info LP.cache.bug_subscription_info object.
+ * @param {Object} extra_data Additional global variables to substitute
+ * in strings. Passed directly through to safely_render_description().
+ * @returns {Object} Y.Node with the ID of 'other-subscriptions' and
+ * add descriptions of each subscription as a separate node.
+ */
+function get_other_descriptions_node(info, extra_data) {
+ var subs = gather_nondirect_subscriptions(info);
+ if (subs.length > 0) {
+ var other_node = Y.Node.create('<div></div>')
+ .set('id', 'other-subscriptions');
+
+ for (var index in subs) {
+ other_node.appendChild(
+ get_single_description_node(subs[index], extra_data));
+ }
+
+ return other_node;
+ } else {
+ return undefined;
+ }
+}
+namespace._get_other_descriptions_node = get_other_descriptions_node;
+
+
+/**
+ * Add descriptions for all non-structural subscriptions to the page.
+ *
+ * @param {Object} config Object specifying the node to populate in
+ * `description_box` and allowing LP.cache.bug_subscription_info
+ * override with `subscription_info` property.
+ */
+function show_subscription_description(config) {
// Allow tests to pass subscription_info directly in.
var info = config.subscription_info || LP.cache.bug_subscription_info;
- console.log(info);
+ // Replace subscription-cache-reference-* strings with actual
+ // object references.
replace_textual_references(info, LP.cache);
- var subs = gather_nondirect_subscriptions(info);
- console.log(subs);
-
+ var extra_data = {
+ bug_id: '#' + info.bug_id.toString(),
+ };
+
+ var content_node = Y.one(config.description_box);
+
+ var direct_node = get_direct_description_node(info);
+ content_node.appendChild(direct_node);
+
+ var other_node = get_other_descriptions_node(info, extra_data);
+ if (other_node !== undefined) {
+ content_node.appendChild(other_node);
+ }
}
-namespace.get_subscription_reason = get_subscription_reason
+namespace.show_subscription_description = show_subscription_description
}, '0.1', {requires: [
'dom', 'node', 'substitute'
=== modified file 'lib/lp/bugs/javascript/tests/test_subscription.html'
--- lib/lp/bugs/javascript/tests/test_subscription.html 2011-04-11 10:00:45 +0000
+++ lib/lp/bugs/javascript/tests/test_subscription.html 2011-04-13 10:56:37 +0000
@@ -34,6 +34,7 @@
</head>
<body class="yui3-skin-sam">
<!-- Example markup required by test suite -->
+ <div id="test-root"></div>
<!-- The test output -->
<div id="log"></div>
=== modified file 'lib/lp/bugs/javascript/tests/test_subscription.js'
--- lib/lp/bugs/javascript/tests/test_subscription.js 2011-04-13 06:26:27 +0000
+++ lib/lp/bugs/javascript/tests/test_subscription.js 2011-04-13 10:56:37 +0000
@@ -588,7 +588,7 @@
Y.Assert.areEqual('My team', subs[0].vars.team.title);
Y.Assert.areEqual('http://link', subs[0].vars.team.url);
- Y.Assert.areEqual('bug #1', subs[0].vars.duplicate_bug.title);
+ Y.Assert.areEqual('#1', subs[0].vars.duplicate_bug.title);
Y.Assert.areEqual(
'http://launchpad/bug/1', subs[0].vars.duplicate_bug.url);
},
@@ -722,6 +722,559 @@
},
}));
+
+/**
+ * Helper to construct a single 'category' of subscriptions,
+ * grouped by type (personally, as team member and as team admin).
+ */
+function _constructCategory(personal, as_member, as_admin) {
+ if (personal === undefined) {
+ personal = [];
+ }
+ if (as_member === undefined) {
+ as_member = [];
+ }
+ if (as_admin === undefined) {
+ as_admin = [];
+ }
+ return {
+ count: personal.length + as_admin.length + as_member.length,
+ personal: personal,
+ as_team_member: as_member,
+ as_team_admin: as_admin
+ };
+}
+
+/**
+ * Get the reason for a direct subscription.
+ * Tests for method get_direct_subscription_information().
+ */
+suite.add(new Y.Test.Case({
+ name: 'Get reason for a direct subscription',
+
+ _should: {
+ error: {
+ test_multiple_direct_subscriptions:
+ new Error('A person should not have more than ' +
+ 'one direct personal subscription.')
+ }
+ },
+
+ test_multiple_direct_subscriptions: function() {
+ // It should not be possible to have multiple direct,
+ // personal subscriptions.
+ // This errors out (see _should.error above).
+ var info = {
+ direct: _constructCategory(['1', '2']),
+ count: 2
+ };
+ module._get_direct_subscription_information(info);
+ },
+
+ test_no_subscriptions: function() {
+ // There are no subscriptions at all.
+ var info = {
+ direct: _constructCategory(),
+ from_duplicates: _constructCategory()
+ };
+ info.count = info.direct.count + info.from_duplicates.count;
+
+ Y.Assert.areEqual(
+ module._reasons.NOT_SUBSCRIBED,
+ module._get_direct_subscription_information(info));
+ },
+
+ test_no_direct_subscriptions: function() {
+ // There is no direct subscription, but there are
+ // other subscriptions.
+ var info = {
+ direct: _constructCategory(),
+ from_duplicates: _constructCategory(['dupe'])
+ };
+ info.count = info.direct.count + info.from_duplicates.count;
+ Y.Assert.areSame(
+ module._reasons.NOT_PERSONALLY_SUBSCRIBED,
+ module._get_direct_subscription_information(info));
+ },
+
+ test_muted_subscription: function() {
+ // The direct subscription is muted.
+ var info = {
+ direct: _constructCategory(['direct']),
+ muted: true,
+ };
+ info.count = info.direct.count;
+ 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,
+ security_related: false,
+ },
+ principal_is_reporter: false,
+ };
+ var info = {
+ direct: _constructCategory([sub]),
+ count: 1
+ };
+
+ Y.Assert.areSame(
+ module._reasons.YOU_SUBSCRIBED,
+ 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,
+ };
+ var info = {
+ direct: _constructCategory([sub]),
+ count: 1
+ };
+ Y.Assert.areSame(
+ module._reasons.YOU_REPORTED,
+ module._get_direct_subscription_information(info));
+ },
+
+ test_direct_subscription_for_supervisor: function() {
+ // The direct subscription created on private bugs for
+ // the bug supervisor.
+ var sub = {
+ bug: {
+ private: true,
+ },
+ };
+ var info = {
+ direct: _constructCategory([sub]),
+ count: 1
+ };
+ Y.Assert.areSame(
+ module._reasons.YOU_SUBSCRIBED_BUG_SUPERVISOR,
+ module._get_direct_subscription_information(info));
+ },
+
+ test_direct_subscription_for_security_contact: function() {
+ // The simple direct subscription.
+ var sub = {
+ bug: {
+ security_related: true,
+ },
+ };
+ var info = {
+ direct: _constructCategory([sub]),
+ count: 1
+ };
+ Y.Assert.areSame(
+ module._reasons.YOU_SUBSCRIBED_SECURITY_CONTACT,
+ module._get_direct_subscription_information(info));
+ },
+
+}));
+
+/**
+ * Test for get_objectlink_html() method.
+ */
+suite.add(new Y.Test.Case({
+ name: 'Test conversion of ObjectLink to HTML.',
+
+ _should: {
+ error: {
+ test_non_link: new Error('Not a proper ObjectLink.')
+ }
+ },
+
+ test_string: function() {
+ // When a string is passed in, it is returned unmodified.
+ var link = 'test';
+ Y.Assert.areEqual(
+ link,
+ module._get_objectlink_html(link));
+ },
+
+ test_non_link: function() {
+ // When an object that doesn't have both 'title' and 'url'
+ // passed in, it fails. (see _should.error above)
+ var link = {};
+ module._get_objectlink_html(link);
+ },
+
+ test_simple: function() {
+ // When a string is passed in, it is returned unmodified.
+ var link = {
+ title: 'Title',
+ url: 'http://url/'
+ };
+ Y.Assert.areEqual(
+ '<a href="http://url/";>Title</a>',
+ module._get_objectlink_html(link));
+ },
+
+ test_escaping_title: function() {
+ // Even with title containing HTML characters, they are properly
+ // escaped.
+ var link = {
+ title: 'Title<script>',
+ url: 'http://url/'
+ };
+ Y.Assert.areEqual(
+ '<a href="http://url/";>Title<script></a>',
+ module._get_objectlink_html(link));
+ },
+
+ test_escaping_url: function() {
+ // Even with title containing HTML characters, they are properly
+ // escaped.
+ var link = {
+ title: 'Title',
+ url: 'http://url/"; onclick="javascript:alert(\'test\');" a="'
+ };
+ Y.Assert.areEqual(
+ '<a href="http://url/" onclick="'+
+ 'javascript:alert(\'test\');" a="">Title</a>',
+ module._get_objectlink_html(link));
+ },
+
+}));
+
+/**
+ * Test for safely_render_description() method.
+ */
+suite.add(new Y.Test.Case({
+ name: 'Test variable substitution in subscription descriptions.',
+
+ _should: {
+ error: {
+ test_non_link: new Error('Not a proper ObjectLink.')
+ }
+ },
+
+ test_no_variables: function() {
+ // For a string with no variables, no substitution is performed.
+ var sub = {
+ reason: 'test string with no vars',
+ vars: { no: 'vars' }
+ };
+
+ Y.Assert.areEqual(
+ sub.reason,
+ module._safely_render_description(sub));
+ },
+
+ test_missing_variable: function() {
+ // If a variable is missing, it is not substituted.
+ var sub = {
+ reason: 'test string with {foo}',
+ vars: {}
+ };
+
+ Y.Assert.areEqual(
+ 'test string with {foo}',
+ module._safely_render_description(sub));
+ },
+
+ test_string_variable: function() {
+ // Plain string variables are directly substituted.
+ var sub = {
+ reason: 'test string with {foo}',
+ vars: { foo: 'nothing' }
+ };
+
+ Y.Assert.areEqual(
+ 'test string with nothing',
+ module._safely_render_description(sub));
+ },
+
+ _constructObjectLink: function(title, url) {
+ // Constructs a mock ObjectLink.
+ return { title: title, url: url };
+ },
+
+ test_objectlink_variable: function() {
+ // ObjectLink variables get turned into actual HTML links.
+ var sub = {
+ reason: 'test string with {foo}',
+ vars: { foo: this._constructObjectLink('Title', 'http://link/') }
+ };
+
+ Y.Assert.areEqual(
+ 'test string with <a href="http://link/";>Title</a>',
+ module._safely_render_description(sub));
+ },
+
+ test_multiple_variables: function() {
+ // For multiple variables, they all get replaced.
+ var sub = {
+ reason: '{simple} string with {foo} {simple}',
+ vars: {
+ foo: this._constructObjectLink('Link', 'http://link/'),
+ simple: "test"
+ }
+ };
+
+ Y.Assert.areEqual(
+ 'test string with <a href="http://link/";>Link</a> test',
+ module._safely_render_description(sub));
+ },
+
+ test_extra_variable: function() {
+ // Passing in extra variables causes them to be replaced as well.
+ var sub = {
+ reason: 'test string with {extra}',
+ vars: {}
+ };
+ var extra_vars = {
+ extra: 'something extra'
+ };
+
+ Y.Assert.areEqual(
+ 'test string with something extra',
+ module._safely_render_description(sub, extra_vars));
+ },
+
+ test_extra_objectlink_variable: function() {
+ // Passing in extra ObjectLink variable gets properly substituted.
+ var sub = {
+ reason: 'test string with {extra}',
+ vars: {}
+ };
+ var extra_vars = {
+ extra: this._constructObjectLink('extras', 'http://link/')
+ };
+
+ Y.Assert.areEqual(
+ 'test string with <a href="http://link/";>extras</a>',
+ module._safely_render_description(sub, extra_vars));
+ },
+
+}));
+
+/**
+ * Test for get_direct_description_node() method.
+ */
+suite.add(new Y.Test.Case({
+ name: 'Test direct node construction with appropriate description.',
+
+ test_no_subscriptions: function() {
+ // A description is added in even when there are no subscriptions.
+ var info = {
+ direct: _constructCategory(),
+ count: 0
+ };
+ var expected_text = module._get_direct_subscription_information(info);
+ var node = module._get_direct_description_node(info);
+ Y.Assert.areEqual(
+ 'direct-subscription', node.get('id'));
+ Y.Assert.areEqual(
+ expected_text, node.get('text'));
+ },
+
+ test_direct_subscription: function() {
+ // One personal, direct subscription exists.
+ var info = {
+ direct: _constructCategory([{ bug: {} }]),
+ count: 1
+ };
+ var expected_text = module._get_direct_subscription_information(info);
+ var node = module._get_direct_description_node(info);
+ Y.Assert.areEqual(
+ 'direct-subscription', node.get('id'));
+ Y.Assert.areEqual(
+ expected_text, node.get('text'));
+ },
+
+}));
+
+/**
+ * Test for get_single_description_node() method.
+ */
+suite.add(new Y.Test.Case({
+ name: 'Test single subscription description node construction.',
+
+ test_simple_text: function() {
+ // A simple subscription with 'Text' as the reason and no variables.
+ var sub = { reason: 'Text', vars: {} };
+ var node = module._get_single_description_node(sub);
+
+ // The node has appropriate CSS class set.
+ Y.Assert.isTrue(node.hasClass('subscription-description'));
+
+ // There is also a sub-node containing the actual description.
+ var subnode = node.one('.description-text');
+ Y.Assert.areEqual('Text', subnode.get('text'));
+ },
+
+ test_variable_substitution: function() {
+ // A subscription with variables and extra variables
+ // has them replaced.
+ var sub = { reason: 'Test {var1} {var2}',
+ vars: { var1: 'my text'} };
+ var extra_data = { var2: 'globally' };
+ var node = module._get_single_description_node(sub, extra_data);
+
+ // The node has appropriate CSS class set.
+ Y.Assert.isTrue(node.hasClass('subscription-description'));
+
+ // There is also a sub-node containing the actual description.
+ var subnode = node.one('.description-text');
+ Y.Assert.areEqual('Test my text globally', subnode.get('text'));
+ },
+
+}));
+
+/**
+ * Test for get_other_descriptions_node() method.
+ */
+suite.add(new Y.Test.Case({
+ name: 'Test creation of node describing all non-direct subscriptions.',
+
+ test_no_subscriptions: function() {
+ // With just a personal subscription, undefined is returned.
+ var info = {
+ direct: _constructCategory([{ bug: {} }]),
+ from_duplicate: _constructCategory(),
+ as_assignee: _constructCategory(),
+ as_owner: _constructCategory(),
+ count: 1
+ };
+ Y.Assert.areSame(
+ undefined,
+ module._get_other_descriptions_node(info));
+ },
+
+ test_one_subscription: function() {
+ // There is a subscription on the duplicate bug.
+ var info = {
+ direct: _constructCategory(),
+ from_duplicate: _constructCategory([{ bug: {id: 1} }]),
+ as_assignee: _constructCategory(),
+ as_owner: _constructCategory(),
+ count: 1
+ };
+
+ // A node is returned with ID of 'other-subscriptions'.
+ var node = module._get_other_descriptions_node(info);
+ Y.Assert.areEqual(
+ 'other-subscriptions', node.get('id'));
+ // And it contains single '.subscription-description' node.
+ Y.Assert.areEqual(
+ 1, node.all('.subscription-description').size());
+ },
+
+ test_multiple_subscription: function() {
+ // There is a subscription on the duplicate bug 1,
+ // and another as assignee on bug 2.
+ var info = {
+ direct: _constructCategory(),
+ from_duplicate: _constructCategory([{ bug: {id: 1} }]),
+ as_assignee: _constructCategory([{ bug: {id: 2} }]),
+ as_owner: _constructCategory(),
+ count: 1
+ };
+
+ // A node is returned containing two
+ // '.subscription-description' nodes.
+ var node = module._get_other_descriptions_node(info);
+ Y.Assert.areEqual(
+ 2, node.all('.subscription-description').size());
+ },
+
+}));
+
+/**
+ * Test for show_subscription_description() method.
+ */
+suite.add(new Y.Test.Case({
+ name: 'Test showing of subscription descriptions.',
+
+ setUp: function() {
+ this.content_node = Y.Node.create('<div></div>')
+ .set('id', 'description-container');
+ this.parent_node = Y.one('#test-root');
+ this.parent_node.appendChild(this.content_node);
+ this.config = {
+ description_box: '#description-container',
+ };
+ },
+
+ tearDown: function() {
+ this.parent_node.empty(true);
+ delete this.config;
+ },
+
+ test_no_subscriptions: function() {
+ // With no subscriptions, a simple description of that state
+ // is added.
+ this.config.subscription_info = {
+ direct: _constructCategory(),
+ from_duplicate: _constructCategory(),
+ as_assignee: _constructCategory(),
+ as_owner: _constructCategory(),
+ bug_id: 1,
+ count: 0
+ };
+ window.LP = {};
+ module.show_subscription_description(this.config);
+ this.wait(function() {
+ Y.Assert.areEqual(
+ 1, this.content_node.all('#direct-subscription').size());
+ Y.Assert.areEqual(
+ 0, this.content_node.all('#other-subscriptions').size());
+ }, 50);
+ },
+
+ test_combined_subscriptions: function() {
+ // With both direct and implicit subscriptions,
+ // we get a simple description and a node with other descriptions.
+ this.config.subscription_info = {
+ direct: _constructCategory([{ bug: {id:1} }]),
+ from_duplicate: _constructCategory([{ bug: {id:2} }]),
+ as_assignee: _constructCategory([{ bug: {id:3} }]),
+ as_owner: _constructCategory(),
+ bug_id: 1,
+ count: 0
+ };
+ window.LP = {};
+ module.show_subscription_description(this.config);
+ this.wait(function() {
+ Y.Assert.areEqual(
+ 1, this.content_node.all('#direct-subscription').size());
+ Y.Assert.areEqual(
+ 1, this.content_node.all('#other-subscriptions').size());
+ }, 50);
+ },
+
+ test_reference_substitutions: function() {
+ // References of the form `subscription-cache-reference-*` get
+ // replaced with LP.cache[...] values.
+ this.config.subscription_info = {
+ reference: 'subscription-cache-reference-X',
+ direct: _constructCategory(),
+ from_duplicate: _constructCategory(),
+ as_assignee: _constructCategory(),
+ as_owner: _constructCategory(),
+ bug_id: 1,
+ count: 0
+ };
+ window.LP = {
+ cache: {
+ 'subscription-cache-reference-X': 'value'
+ }
+ };
+ module.show_subscription_description(this.config);
+ Y.Assert.areEqual(
+ 'value',
+ this.config.subscription_info.reference);
+ },
+
+}));
+
var handle_complete = function(data) {
status_node = Y.Node.create(
'<p id="complete">Test status: complete</p>');
=== modified file 'lib/lp/bugs/model/personsubscriptioninfo.py'
--- lib/lp/bugs/model/personsubscriptioninfo.py 2011-04-08 13:40:12 +0000
+++ lib/lp/bugs/model/personsubscriptioninfo.py 2011-04-13 10:56:37 +0000
@@ -297,7 +297,8 @@
'as_owner': as_owner,
'as_assignee': as_assignee,
'count': self.count,
- 'muted': self.muted
+ 'muted': self.muted,
+ 'bug_id': self.bug.id,
}
for category, collection in ((as_owner, self.as_owner),
(as_assignee, self.as_assignee)):
=== modified file 'lib/lp/bugs/model/tests/test_personsubscriptioninfo.py'
--- lib/lp/bugs/model/tests/test_personsubscriptioninfo.py 2011-04-07 21:31:29 +0000
+++ lib/lp/bugs/model/tests/test_personsubscriptioninfo.py 2011-04-13 10:56:37 +0000
@@ -110,6 +110,7 @@
self.assertEqual(subscriptions['from_duplicate']['count'], 0)
self.assertEqual(subscriptions['as_owner']['count'], 0)
self.assertEqual(subscriptions['as_assignee']['count'], 0)
+ self.assertEqual(subscriptions['bug_id'], self.bug.id)
def test_assignee(self):
with person_logged_in(self.subscriber):
=== modified file 'lib/lp/bugs/templates/bug-subscription-list.pt'
--- lib/lp/bugs/templates/bug-subscription-list.pt 2011-04-08 10:35:12 +0000
+++ lib/lp/bugs/templates/bug-subscription-list.pt 2011-04-13 10:56:37 +0000
@@ -22,8 +22,8 @@
Y.on('domready', function() {
ss_module.setup_bug_subscriptions(
{content_box: "#structural-subscription-content-box"});
- console.log(info_module.get_subscription_reason(
- {description_box: "#description"}));
+ console.log(info_module.show_subscription_description(
+ {description_box: "#subscription-description-box"}));
});
});
</script>
@@ -35,6 +35,7 @@
<div id="maincontent">
<div id="nonportlets" class="readable">
+ <div id="subscription-description-box"></div>
<div id="subscription-listing"></div>
<div id="structural-subscription-content-box"></div>
