Merge lp:~danilo/launchpad/bug-772754-other-subscribers-actions into lp:launchpad

[Данило Шеган]

= Bug 772754: Other subscribers list, part 6 =

Warning: slightly over-sized as well, mostly for the long JS tests which emulate lp_client behaviour.

This is part of ongoing work for providing the "other subscribers" list as indicated in mockup https://launchpadlibrarian.net/71552495/all-in-one.png attached to bug 772754 by Gary.

Provides subscribe-someone-else and unsubscribe actions that actually work with Launchpad.

Existing list of subscribers is only removed in the next branch, so with this branch, you have two lists of which only this one will now really work.

== Tests ==

lp/bugs/javascript/tests/test_subscribers_list.html

bin/test -cvvt BugPortletSubscribersWithDetailsTests

== Demo and Q/A ==

N/A

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/bugs/browser/bugsubscription.py
  lib/lp/bugs/browser/tests/test_bugsubscription_views.py
  lib/lp/bugs/javascript/bugtask_index_portlets.js
  lib/lp/bugs/javascript/subscribers_list.js
  lib/lp/bugs/javascript/tests/test_subscribers_list.html
  lib/lp/bugs/javascript/tests/test_subscribers_list.js
  lib/lp/bugs/templates/bugtask-index.pt

[Brad Crittenden (bac)]

Hi Danilo this branch looks good.

review: Approve (code)

=== modified file 'lib/lp/bugs/browser/bugsubscription.py'
--- lib/lp/bugs/browser/bugsubscription.py	2011-06-15 11:04:53 +0000
+++ lib/lp/bugs/browser/bugsubscription.py	2011-06-15 11:05:00 +0000
@@ -16,7 +16,10 @@
 import cgi
 
 from lazr.delegates import delegates
-from lazr.restful.interfaces import IJSONRequestCache
+from lazr.restful.interfaces import (
+    IJSONRequestCache,
+    IWebServiceClientRequest,
+)
 from simplejson import dumps
 from zope import formlib
 from zope.app.form import CustomWidgetFactory
@@ -27,6 +30,7 @@
     SimpleVocabulary,
     )
 from zope.security.proxy import removeSecurityProxy
+from zope.traversing.browser import absoluteURL
 
 from canonical.launchpad import _
 from canonical.launchpad.webapp import (
@@ -590,6 +594,7 @@
         """Return subscriber_ids in a form suitable for JavaScript use."""
         data = []
         details = list(self.context.getDirectSubscribersWithDetails())
+        api_request = IWebServiceClientRequest(self.request)
         for person, subscription in details:
             if person == self.user:
                 # Skip the current user viewing the page.
@@ -599,6 +604,7 @@
                 'name': person.name,
                 'display_name': person.displayname,
                 'web_link': canonical_url(person, rootsite='mainsite'),
+                'self_link': absoluteURL(person, api_request),
                 'is_team': person.is_team,
                 'can_edit': can_edit,
                 }
@@ -618,6 +624,7 @@
                 'name': person.name,
                 'display_name': person.displayname,
                 'web_link': canonical_url(person, rootsite='mainsite'),
+                'self_link': absoluteURL(person, api_request),
                 'is_team': person.is_team,
                 'can_edit': False,
                 }

=== modified file 'lib/lp/bugs/browser/tests/test_bugsubscription_views.py'
--- lib/lp/bugs/browser/tests/test_bugsubscription_views.py	2011-06-15 11:04:53 +0000
+++ lib/lp/bugs/browser/tests/test_bugsubscription_views.py	2011-06-15 11:05:00 +0000
@@ -7,10 +7,12 @@
 
 from simplejson import dumps
 
+from zope.traversing.browser import absoluteURL
+
 from canonical.launchpad.ftests import LaunchpadFormHarness
 from canonical.launchpad.webapp import canonical_url
 from canonical.testing.layers import LaunchpadFunctionalLayer
-
+from lazr.restful.interfaces import IWebServiceClientRequest
 from lp.bugs.browser.bugsubscription import (
     BugPortletSubcribersIds,
     BugPortletSubscribersWithDetails,
@@ -523,6 +525,7 @@
             bug.subscribe(subscriber, subscriber,
                           level=BugNotificationLevel.LIFECYCLE)
         harness = LaunchpadFormHarness(bug, BugPortletSubscribersWithDetails)
+        api_request = IWebServiceClientRequest(harness.request)
 
         expected_result = {
             'subscriber': {
@@ -531,6 +534,7 @@
                 'is_team': False,
                 'can_edit': False,
                 'web_link': canonical_url(subscriber),
+                'self_link': absoluteURL(subscriber, api_request),
                 },
             'subscription_level': "Lifecycle",
             }
@@ -547,6 +551,7 @@
             bug.subscribe(subscriber, subscriber.teamowner,
                           level=BugNotificationLevel.LIFECYCLE)
         harness = LaunchpadFormHarness(bug, BugPortletSubscribersWithDetails)
+        api_request = IWebServiceClientRequest(harness.request)
 
         expected_result = {
             'subscriber': {
@@ -555,6 +560,7 @@
                 'is_team': True,
                 'can_edit': False,
                 'web_link': canonical_url(subscriber),
+                'self_link': absoluteURL(subscriber, api_request),
                 },
             'subscription_level': "Lifecycle",
             }
@@ -572,6 +578,7 @@
                           level=BugNotificationLevel.LIFECYCLE)
             harness = LaunchpadFormHarness(
                 bug, BugPortletSubscribersWithDetails)
+        api_request = IWebServiceClientRequest(harness.request)
 
         expected_result = {
             'subscriber': {
@@ -580,6 +587,7 @@
                 'is_team': True,
                 'can_edit': True,
                 'web_link': canonical_url(subscriber),
+                'self_link': absoluteURL(subscriber, api_request),
                 },
             'subscription_level': "Lifecycle",
             }
@@ -599,6 +607,7 @@
                           level=BugNotificationLevel.LIFECYCLE)
         harness = LaunchpadFormHarness(
             bug, BugPortletSubscribersWithDetails)
+        api_request = IWebServiceClientRequest(harness.request)
 
         expected_result = {
             'subscriber': {
@@ -607,6 +616,7 @@
                 'is_team': True,
                 'can_edit': True,
                 'web_link': canonical_url(subscriber),
+                'self_link': absoluteURL(subscriber, api_request),
                 },
             'subscription_level': "Lifecycle",
             }

=== modified file 'lib/lp/bugs/javascript/bugtask_index_portlets.js'
--- lib/lp/bugs/javascript/bugtask_index_portlets.js	2011-06-15 11:04:53 +0000
+++ lib/lp/bugs/javascript/bugtask_index_portlets.js	2011-06-15 11:05:00 +0000
@@ -309,7 +309,6 @@
 
     var subscription = get_subscribe_self_subscription();
 
-    setup_subscribe_someone_else_handler(subscription);
 }
 
 function load_subscribers_from_duplicates() {

=== modified file 'lib/lp/bugs/javascript/subscribers_list.js'
--- lib/lp/bugs/javascript/subscribers_list.js	2011-06-15 11:04:53 +0000
+++ lib/lp/bugs/javascript/subscribers_list.js	2011-06-15 11:05:00 +0000
@@ -1,7 +1,23 @@
 /* Copyright 2011 Canonical Ltd.  This software is licensed under the
  * GNU Affero General Public License version 3 (see the file LICENSE).
  *
- * Functions for managing the subscribers list.
+ * Classes for managing the subscribers list.
+ *
+ * Two classes are provided:
+ *
+ *   - SubscribersList: deals with node construction/removal for the
+ *     list of subscribers, including activity indication and animations.
+ *
+ *     Public methods to use:
+ *       startActivity, stopActivity,
+ *       addSubscriber, removeSubscriber, indicateSubscriberActivity,
+ *       stopSubscriberActivity, addUnsubscribeAction
+ *
+ *   - BugSubscribersLoader: loads subscribers from LP, allows subscribing
+ *     someone else and sets unsubscribe actions where appropriate.
+ *     Depends on the SubscribersList to do the actual node construction.
+ *
+ *     No public methods are available: it all gets run from the constructor.
  *
  * @module bugs
  * @submodule subscribers_list
@@ -137,6 +153,7 @@
  */
 function BugSubscribersLoader(config) {
     var sl = this.subscribers_list = new SubscribersList(config);
+
     if (!Y.Lang.isValue(config.bug) ||
         !Y.Lang.isString(config.bug.web_link)) {
         Y.error(
@@ -144,6 +161,7 @@
     }
     this.bug = config.bug;
 
+    // Get BugSubscribersWithDetails portlet link to load subscribers from.
     if (!Y.Lang.isString(config.subscribers_details_view)) {
         Y.error(
             "No config.subscribers_details_view specified to load " +
@@ -165,6 +183,12 @@
     }
 
     this._loadSubscribers();
+
+    // Check for CSS class for the link to subscribe someone else.
+    if (Y.Lang.isString(config.subscribe_someone_else_link)) {
+        this.subscribe_someone_else_link = config.subscribe_someone_else_link;
+        this._setupSubscribeSomeoneElse();
+    }
 }
 namespace.BugSubscribersLoader = BugSubscribersLoader;
 
@@ -268,19 +292,157 @@
  * Return a function object that accepts SubscribersList and subscriber
  * objects as parameters.
  *
+ * Constructed function tries to unsubscribe subscriber from the
+ * this.bug, and indicates activity in the subscribers list.
+ *
  * @method _getUnsubscribeCallback
  */
 BugSubscribersLoader.prototype._getUnsubscribeCallback = function() {
+    var loader = this;
     return function(subscribers_list, subscriber) {
-        subscribers_list.indicateSubscriberActivity(subscriber);
-        // Simulated unsubscribing action for prototyping the UI.
-        setTimeout(function() {
+        function on_success() {
             subscribers_list.stopSubscriberActivity(
                 subscriber, true, function() {
-                    subscribers_list.removeSubscriber(subscriber);
-                });
-        }, 2400);
-    };
+                subscribers_list.removeSubscriber(subscriber);
+            });
+        }
+        function on_failure(t_id, response) {
+            subscribers_list.stopSubscriberActivity(subscriber, false);
+            Y.lp.app.errors.display_error(
+                false,
+                response.status + " (" + response.statusText + ")."
+            );
+        }
+
+        var config = {
+            on: { success: on_success,
+                  failure: on_failure },
+            parameters: { person: subscriber.self_link }
+        };
+        subscribers_list.indicateSubscriberActivity(subscriber);
+        loader.lp_client.named_post(
+            loader.bug.self_link, 'unsubscribe', config);
+    };
+};
+
+/**
+ * Set-up subscribe-someone-else link to pop-up a picker and subscribe
+ * the selected person/team.
+ *
+ * On `save' from the picker, fetch the actual person object via API
+ * and pass it into _subscribeSomeoneElse().
+ *
+ * @method _setupSubscribeSomeoneElse
+ */
+BugSubscribersLoader.prototype._setupSubscribeSomeoneElse = function() {
+    var loader = this;
+    var config = {
+        header: 'Subscribe someone else',
+        step_title: 'Search',
+        picker_activator: this.subscribe_someone_else_link
+    };
+    if (Y.one(this.subscribe_someone_else_link) === null) {
+        Y.error("No link matching CSS selector '" +
+                this.subscribe_someone_else_link +
+                "' for subscribing someone else found.");
+    }
+    config.save = function(result) {
+        var person_uri = Y.lp.client.get_absolute_uri(result.api_uri);
+        loader.lp_client.get(person_uri, {
+            on: {
+                success: function(person) {
+                    loader._subscribeSomeoneElse(person);
+                },
+                failure: function(t_id, response) {
+                    Y.lp.app.errors.display_error(
+                        false,
+                        response.status + " (" + response.statusText + ")\n" +
+                            "Couldn't get subscriber details from the " +
+                            "server, so they have not been subscribed.\n"
+                    );
+                }
+            } });
+    };
+    // We store the picker for testing only.
+    this._picker = Y.lp.app.picker.create('ValidPersonOrTeam', config);
+};
+
+/**
+ * Subscribe a person or a team to the bug.
+ *
+ * This is a callback for the subscribe someone else picker.
+ *
+ * @method _subscribeSomeoneElse
+ * @param person {Object} Representation of a person returned by the API.
+ *     It's an object that returns all attributes with getAttrs() method.
+ *     Must have at least self_link attribute which is passed as
+ *     a parameter to the API 'unsubscribe' call.
+ */
+BugSubscribersLoader.prototype._subscribeSomeoneElse = function(person) {
+    var subscriber = person.getAttrs();
+    this.subscribers_list.addSubscriber(subscriber, 'Discussion');
+    this.subscribers_list.indicateSubscriberActivity(subscriber);
+
+    var loader = this;
+
+    function on_success() {
+        loader.subscribers_list.stopSubscriberActivity(subscriber, true);
+        loader._addUnsubscribeLinkIfTeamMember(subscriber);
+    }
+    function on_failure(t_id, response) {
+        loader.subscribers_list.stopSubscriberActivity(
+            subscriber, false, function() {
+                loader.subscribers_list.removeSubscriber(subscriber);
+            }
+        );
+        Y.lp.app.errors.display_error(
+            false,
+            response.status + " (" + response.statusText + "). " +
+                "Failed to subscribe " + subscriber.display_name + "."
+        );
+    }
+    var config = {
+        on: { success: on_success,
+              failure: on_failure },
+        parameters: { person: subscriber.self_link } };
+    this.lp_client.named_post(this.bug.self_link, 'subscribe', config);
+};
+
+/**
+ * Add unsubscribe link for a team if the currently logged in user
+ * is member of the team.
+ *
+ * @method _addUnsubscribeLinkIfTeamMember
+ * @param team {Object} A person object as returned via API.
+ */
+BugSubscribersLoader.prototype
+._addUnsubscribeLinkIfTeamMember = function(team) {
+    var loader = this;
+    function on_success(members) {
+        var team_member = false;
+        var i;
+        for (i=0; i<members.entries.length; i++) {
+            if (members.entries[i].get('member_link') ===
+                Y.lp.client.get_absolute_uri(LP.links.me)) {
+                team_member = true;
+                break;
+            }
+        }
+        if (team_member === true) {
+            // Add unsubscribe action for the team member.
+            loader.subscribers_list.addUnsubscribeAction(
+                team, loader._getUnsubscribeCallback());
+        }
+    }
+
+    if (Y.Lang.isString(LP.links.me) && team.is_team) {
+        var config = {
+            on: { success: on_success }
+        };
+
+        var members_link = team.members_details_collection_link;
+        this.lp_client.get(members_link, config);
+    }
 };
 
 
@@ -902,4 +1064,5 @@
 };
 
 
-}, "0.1", {"requires": ["node", "lazr.anim", "lp.client", "lp.names"]});
+}, "0.1", {"requires": ["node", "lazr.anim", "lp.app.picker", "lp.app.errors",
+                        "lp.client", "lp.names"]});

=== modified file 'lib/lp/bugs/javascript/tests/test_subscribers_list.html'
--- lib/lp/bugs/javascript/tests/test_subscribers_list.html	2011-06-15 11:04:53 +0000
+++ lib/lp/bugs/javascript/tests/test_subscribers_list.html	2011-06-15 11:05:00 +0000
@@ -22,6 +22,10 @@
       src="../../../app/javascript/errors.js"></script>
     <script type="text/javascript"
       src="../../../app/javascript/lp-names.js"></script>
+    <script type="text/javascript"
+      src="../../../app/javascript/picker.js"></script>
+    <script type="text/javascript"
+      src="../../../app/javascript/widgets.js"></script>
 
     <!-- Pre-requisite -->
     <script type="text/javascript"

=== modified file 'lib/lp/bugs/javascript/tests/test_subscribers_list.js'
--- lib/lp/bugs/javascript/tests/test_subscribers_list.js	2011-06-15 11:04:53 +0000
+++ lib/lp/bugs/javascript/tests/test_subscribers_list.js	2011-06-15 11:05:00 +0000
@@ -1,9 +1,9 @@
 YUI({
     base: '../../../../canonical/launchpad/icing/yui/',
     filter: 'raw', combine: false, fetchCSS: false
-    }).use('test', 'console', 'lp.bugs.subscriber',
-           'lp.bugs.subscribers_list', 'node-event-simulate',
-           function(Y) {
+}).use('test', 'console', 'lp.app.picker', 'lp.bugs.subscriber',
+       'lp.bugs.subscribers_list', 'node-event-simulate',
+       function(Y) {
 
 var suite = new Y.Test.Suite("lp.bugs.subscribers_list Tests");
 var module = Y.lp.bugs.subscribers_list;
@@ -1596,7 +1596,7 @@
         container_box: '#other-subscribers-container'
     };
     if (barebone !== true) {
-        container_config.bug = { web_link: '/base' };
+        container_config.bug = { web_link: '/base', self_link: '/bug/1' };
         container_config.subscribers_details_view = '/+details';
     }
     root_node.appendChild(node);
@@ -1937,6 +1937,485 @@
 }));
 
 
+
+/**
+ * Test BugSubscribersLoader unsubscribe callback function.
+ */
+suite.add(new Y.Test.Case({
+    name: 'BugSubscribersLoader() subscribers loading test',
+
+    setUp: function() {
+        this.root = Y.Node.create('<div />');
+        Y.one('body').appendChild(this.root);
+    },
+
+    tearDown: function() {
+        this.root.remove();
+    },
+
+    test_unsubscribe_callback_success: function() {
+        // _getUnsubscribeCallback returns a function which takes
+        // subscribers list and subscriber as the two parameters.
+        // That function calls 'unsubscribe' API method on the bug
+        // to unsubscribe the user, and on successful completion,
+        // it removes the user from the subscribers list.
+
+        // Mock LP client.
+        var received_uri, received_method, received_params;
+        var config = {};
+        config.lp_client = {
+            named_post: function(uri, method, my_conf) {
+                received_uri = uri;
+                received_method = method;
+                received_params = my_conf.parameters;
+                my_conf.on.success();
+            },
+            get: function() {}
+        };
+        var subscriber = { name: "user", "can_edit": true,
+                           self_link: "user-self-link" };
+
+        // Mock removeSubscriber method to ensure it's called.
+        var removed_subscriber = false;
+        var old_rmSub = module.SubscribersList.prototype.removeSubscriber;
+        module.SubscribersList.prototype.removeSubscriber = function(
+            my_subscriber) {
+            Y.Assert.areSame(subscriber.name, my_subscriber.name);
+            removed_subscriber = true;
+        };
+
+        var loader = setUpLoader(this.root, config);
+        var unsub_callback = loader._getUnsubscribeCallback();
+        loader._addSubscriber(subscriber);
+        unsub_callback(loader.subscribers_list, subscriber);
+
+        Y.Assert.areSame(loader.bug.self_link, received_uri);
+        Y.Assert.areSame('unsubscribe', received_method);
+        Y.Assert.areSame(subscriber.self_link, received_params.person);
+
+        this.wait(function() {
+            // Removal is triggered from the stopSubscriberActivity,
+            // which shows the success animation first.
+            Y.Assert.isTrue(removed_subscriber);
+        }, 1100);
+
+        // Restore the real method.
+        module.SubscribersList.prototype.removeSubscriber = old_rmSub;
+    },
+
+    test_unsubscribe_callback_failure: function() {
+        // Function returned by _getUnsubscribeCallback calls
+        // 'unsubscribe' API method on the bug, and on failure,
+        // it keeps the user in the list and calls
+        // stopSubscriberActivity to indicate the failure.
+
+        // Mock LP client.
+        var config = {};
+        config.lp_client = {
+            named_post: function(uri, method, my_conf) {
+                my_conf.on.failure(0, { status: 500, statusText: "BOOM!" });
+            },
+            get: function() {}
+        };
+        var subscriber = { name: "user", "can_edit": true,
+                           self_link: "user-self-link" };
+
+        // Mock stopSubscriberActivity to ensure it's called.
+        var subscriber_activity_stopped = false;
+        var old_method =
+            module.SubscribersList.prototype.stopSubscriberActivity;
+        module.SubscribersList.prototype.stopSubscriberActivity = function(
+            my_subscriber, success, callback) {
+            Y.Assert.areSame(subscriber.name, my_subscriber.name);
+            // The passed-in parameter indicates failure.
+            Y.Assert.isFalse(success);
+            // And there is no callback.
+            Y.Assert.isUndefined(callback);
+            subscriber_activity_stopped = true;
+        };
+
+        // Ensure display_error is called.
+        var error_shown = false;
+        var old_error_method = Y.lp.app.errors.display_error;
+        Y.lp.app.errors.display_error = function(text) {
+            error_shown = true;
+        };
+
+        var loader = setUpLoader(this.root, config);
+        var unsub_callback = loader._getUnsubscribeCallback();
+        loader._addSubscriber(subscriber);
+        unsub_callback(loader.subscribers_list, subscriber);
+
+        Y.Assert.isTrue(subscriber_activity_stopped);
+        Y.Assert.isTrue(error_shown);
+
+        // Restore original methods.
+        module.SubscribersList.prototype.stopSubscriberActivity = old_method;
+        Y.lp.app.errors.display_error = old_error_method;
+    }
+
+}));
+
+
+
+/**
+ * Test BugSubscribersLoader subscribe-someone-else functionality.
+ */
+suite.add(new Y.Test.Case({
+    name: 'BugSubscribersLoader() subscribe-someone-else test',
+
+    _should: {
+        error: {
+            test_setupSubscribeSomeoneElse_error:
+            new Error("No link matching CSS selector " +
+                      "'#sub-someone-else-link' " +
+                      "for subscribing someone else found.")
+        }
+    },
+
+    setUp: function() {
+        this.root = Y.Node.create('<div />');
+        Y.one('body').appendChild(this.root);
+    },
+
+    tearDown: function() {
+        this.root.remove();
+    },
+
+    test_constructor_calls_setup: function() {
+        // When subscribe_someone_else_link is passed in the constructor,
+        // link identified by that CSS selector is set to pop up a person
+        // picker for choosing a person/team to subscribe to the bug.
+        var config = {
+            subscribe_someone_else_link: '#sub-someone-else-link'
+        };
+
+        var setup_called = false;
+        // Replace the original method to ensure it's getting called.
+        var old_method =
+            module.BugSubscribersLoader.prototype._setupSubscribeSomeoneElse;
+        module.BugSubscribersLoader.prototype._setupSubscribeSomeoneElse =
+            function() {
+                setup_called = true;
+            };
+
+        var loader = setUpLoader(this.root, config);
+
+        Y.Assert.isTrue(setup_called);
+
+        // Restore original method.
+        module.BugSubscribersLoader.prototype._setupSubscribeSomeoneElse =
+            old_method;
+    },
+
+    test_setupSubscribeSomeoneElse_error: function() {
+        // When link is not found in the page, exception is raised.
+
+        // Initialize the loader with no subscribe-someone-else link.
+        var loader = setUpLoader(this.root);
+        loader.subscribe_someone_else_link = '#sub-someone-else-link';
+        loader._setupSubscribeSomeoneElse();
+    },
+
+    test_setupSubscribeSomeoneElse: function() {
+        // _setupSubscribeSomeoneElse ties in a link with
+        // the appropriate person picker and with the save
+        // handler that calls _subscribeSomeoneElse with the
+        // selected person as the parameter.
+
+        // Initialize the loader with no subscribe-someone-else link.
+        var loader = setUpLoader(this.root);
+
+        // Mock LP client that always returns a person-like object.
+        var subscriber = { name: "user", "can_edit": true,
+                           self_link: "/~user",
+                           api_uri: "/~user" };
+        loader.lp_client = {
+            get: function(uri, conf) {
+                conf.on.success(subscriber);
+            }
+        };
+
+        loader.subscribe_someone_else_link = '#sub-someone-else-link';
+        var link = Y.Node.create('<a />').set('id', 'sub-someone-else-link');
+        this.root.appendChild(link);
+
+        // Mock subscribeSomeoneElse method to ensure it's called.
+        var subscribe_done = false;
+        var old_method =
+            module.BugSubscribersLoader.prototype._subscribeSomeoneElse;
+        module.BugSubscribersLoader.prototype._subscribeSomeoneElse =
+            function(person) {
+                Y.Assert.areSame(subscriber, person);
+                subscribe_done = true;
+            };
+
+        // Mock the picker creation as well.
+        var picker_shown = false;
+        var old_create_picker = Y.lp.app.picker.create;
+        Y.lp.app.picker.create = function(vocabulary, my_config) {
+            Y.Assert.areSame('ValidPersonOrTeam', vocabulary);
+            // On link click, simulate the save action.
+            link.on('click', function() {
+                picker_shown = true;
+                my_config.save(subscriber);
+            });
+        };
+
+        loader._setupSubscribeSomeoneElse();
+
+        // Show the picker and simulate the save action.
+        link.simulate('click');
+
+        Y.Assert.isTrue(picker_shown);
+        Y.Assert.isTrue(subscribe_done);
+
+        // Restore original methods.
+        module.BugSubscribersLoader.prototype._subscribeSomeoneElse =
+            old_method;
+        Y.lp.app.picker.create = old_create_picker;
+    },
+
+    test_setupSubscribeSomeoneElse_failure: function() {
+        // When fetching a person as returned by the picker fails
+        // error message is shown.
+
+        // Initialize the loader with no subscribe-someone-else link.
+        var loader = setUpLoader(this.root);
+
+        // Mock LP client that always returns a person-like object.
+        var subscriber = { name: "user", "can_edit": true,
+                           self_link: "/~user",
+                           api_uri: "/~user" };
+        loader.lp_client = {
+            get: function(uri, conf) {
+                conf.on.failure(99, { status: 500,
+                                      statusText: "BOOM" });
+            }
+        };
+        var expected_error_msg = "500 (BOOM)\n" +
+            "Couldn't get subscriber details from the " +
+            "server, so they have not been subscribed.\n";
+        var received_error_msg;
+
+        // Mock display_error to ensure it's called.
+        var old_display_error = Y.lp.app.errors.display_error;
+        Y.lp.app.errors.display_error = function(animate, msg) {
+            Y.Assert.isFalse(animate);
+            received_error_msg = msg;
+        };
+
+        loader.subscribe_someone_else_link = '#sub-someone-else-link';
+        var link = Y.Node.create('<a />').set('id', 'sub-someone-else-link');
+        this.root.appendChild(link);
+
+        // Mock the picker creation as well.
+        var old_create_picker = Y.lp.app.picker.create;
+        Y.lp.app.picker.create = function(vocabulary, my_config) {
+            Y.Assert.areSame('ValidPersonOrTeam', vocabulary);
+            // On link click, simulate the save action.
+            link.on('click', function() {
+                my_config.save(subscriber);
+            });
+        };
+
+        loader._setupSubscribeSomeoneElse();
+
+        // Show the picker and simulate the save action.
+        link.simulate('click');
+
+        // display_error was called with the appropriate error message.
+        Y.Assert.areSame(expected_error_msg, received_error_msg);
+
+        // Restore original methods.
+        Y.lp.app.errors.display_error = old_display_error;
+        Y.lp.app.picker.create = old_create_picker;
+    },
+
+    test_subscribeSomeoneElse: function() {
+        // _subscribeSomeoneElse method takes a Person object as returned
+        // by the API, and adds that subscriber at 'Discussion' level.
+
+        var subscriber = { self_link: "/~user" };
+
+        // Mock-up addSubscriber method to ensure subscriber is added.
+        var subscriber_added = false;
+        var old_addSub = module.SubscribersList.prototype.addSubscriber;
+        module.SubscribersList.prototype.addSubscriber = function(
+            my_subscriber, level) {
+            Y.Assert.areSame(subscriber, my_subscriber);
+            subscriber_added = true;
+        };
+
+        // Mock-up indicateSubscriberActivity to ensure it's called.
+        var activity_on = false;
+        var old_indicate =
+            module.SubscribersList.prototype.indicateSubscriberActivity;
+        module.SubscribersList.prototype.indicateSubscriberActivity =
+            function(my_subscriber) {
+                Y.Assert.areSame(subscriber, my_subscriber);
+                activity_on = true;
+            };
+
+        // Initialize the loader.
+        var loader = setUpLoader(this.root);
+
+        // Mock lp_client which records the call.
+        var received_method, received_uri, received_params;
+        loader.lp_client = {
+            named_post: function(uri, method, conf) {
+                received_uri = uri;
+                received_method = method;
+                received_params = conf.parameters;
+            }
+        };
+
+        // Wrap subscriber like an API-returned value.
+        var person = {
+            getAttrs: function() {
+                return subscriber;
+            }
+        };
+
+        loader._subscribeSomeoneElse(person);
+
+        Y.Assert.isTrue(subscriber_added);
+        Y.Assert.isTrue(activity_on);
+
+        Y.Assert.areEqual('subscribe', received_method);
+        Y.Assert.areEqual(loader.bug.self_link, received_uri);
+        Y.Assert.areEqual(subscriber.self_link, received_params.person);
+
+        // Restore original methods.
+        module.SubscribersList.prototype.addSubscriber = old_addSub;
+        module.SubscribersList.prototype.indicateSubscriberActivity =
+            old_indicate;
+    },
+
+    test_subscribeSomeoneElse_success: function() {
+        // When subscribing someone else succeeds, stopSubscriberActivity
+        // is called indicating success, and _addUnsubscribeLinkIfTeamMember
+        // is called to add unsubscribe-link when needed.
+
+        var subscriber = { name: "user", self_link: "/~user" };
+
+        // Initialize the loader.
+        var loader = setUpLoader(this.root);
+        loader.subscribers_list.addSubscriber(subscriber, "Maybe");
+
+        // Mock-up addUnsubscribeLinkIfTeamMember method to
+        // ensure it's called with the right parameters.
+        var subscriber_link_added = false;
+        var old_addLink =
+            module.BugSubscribersLoader
+            .prototype._addUnsubscribeLinkIfTeamMember;
+        module.BugSubscribersLoader.prototype
+            ._addUnsubscribeLinkIfTeamMember =
+            function(my_subscriber) {
+                Y.Assert.areSame(subscriber, my_subscriber);
+                subscriber_link_added = true;
+            };
+
+        // Mock-up stopSubscriberActivity to ensure it's called.
+        var activity_on = true;
+        var old_indicate =
+            module.SubscribersList.prototype.stopSubscriberActivity;
+        module.SubscribersList.prototype.stopSubscriberActivity =
+            function(my_subscriber, success) {
+                Y.Assert.areSame(subscriber, my_subscriber);
+                Y.Assert.isTrue(success);
+                activity_on = false;
+            };
+
+        // Mock lp_client which calls the success handler.
+        loader.lp_client = {
+            named_post: function(uri, method, conf) {
+                conf.on.success();
+            }
+        };
+
+        // Wrap subscriber like an API-returned value.
+        var person = {
+            getAttrs: function() {
+                return subscriber;
+            }
+        };
+
+        loader._subscribeSomeoneElse(person);
+
+        Y.Assert.isTrue(subscriber_link_added);
+        Y.Assert.isFalse(activity_on);
+
+        // Restore original methods.
+        module.BugSubscribersLoader.prototype.addUnsubscribeLinkIfTeamMember =
+            old_addLink;
+        module.SubscribersList.prototype.stopSubscriberActivity =
+            old_indicate;
+
+    },
+
+    test_subscribeSomeoneElse_failure: function() {
+        // When subscribing someone else fails, stopSubscriberActivity
+        // is called indicating failure and it calls removeSubscriber
+        // from the callback when animation completes.
+        // Error is shown as well.
+
+        var subscriber = { name: "user", self_link: "/~user",
+                           display_name: "User Name" };
+
+        // Initialize the loader.
+        var loader = setUpLoader(this.root);
+        loader.subscribers_list.addSubscriber(subscriber, "Maybe");
+
+        // Mock-up removeSubscriber to ensure it's called.
+        var remove_called = false;
+        var old_remove =
+            module.SubscribersList.prototype.removeSubscriber;
+        module.SubscribersList.prototype.removeSubscriber =
+            function(my_subscriber) {
+                Y.Assert.areSame(subscriber, my_subscriber);
+                remove_called = true;
+            };
+
+        // Ensure display_error is called.
+        var old_error_method = Y.lp.app.errors.display_error;
+        var received_error;
+        Y.lp.app.errors.display_error = function(anim, text) {
+            received_error = text;
+        };
+
+        // Mock lp_client which calls the failure handler.
+        loader.lp_client = {
+            named_post: function(uri, method, conf) {
+                conf.on.failure(99, { status: 500,
+                                      statusText: "BOOM" });
+            }
+        };
+
+        // Wrap subscriber like an API-returned value.
+        var person = {
+            getAttrs: function() {
+                return subscriber;
+            }
+        };
+
+        loader._subscribeSomeoneElse(person);
+
+        Y.Assert.areSame('500 (BOOM). Failed to subscribe User Name.',
+                         received_error);
+
+        // Remove function is only called after animation completes.
+        this.wait(function() {
+            Y.Assert.isTrue(remove_called);
+        }, 1100);
+
+        // Restore original methods.
+        module.SubscribersList.prototype.removeSubscriber = old_remove;
+        Y.lp.app.errors.display_error = old_error_method;
+
+    }
+}));
+
 var handle_complete = function(data) {
     window.status = '::::' + JSON.stringify(data);
     };

=== modified file 'lib/lp/bugs/templates/bugtask-index.pt'
--- lib/lp/bugs/templates/bugtask-index.pt	2011-06-15 11:04:53 +0000
+++ lib/lp/bugs/templates/bugtask-index.pt	2011-06-15 11:05:00 +0000
@@ -59,7 +59,8 @@
                     container_box: '#other-bug-subscribers',
                     bug: LP.cache.bug,
                     subscribers_details_view:
-                        '/+bug-portlet-subscribers-details'
+                        '/+bug-portlet-subscribers-details',
+                    subscribe_someone_else_link: '.menu-link-addsubscriber'
                 });
             });
          });