launchpad-reviewers team mailing list archive

[Данило Шеган <danilo@xxxxxxxxxxxxx>]

Данило Шеган has proposed merging lp:~danilo/launchpad/bug-772754-other-subscribers-actions into lp:launchpad with lp:~danilo/launchpad/bug-772754-other-subscribers-loading as a prerequisite.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #772754 in Launchpad itself: "After better-bug-notification changes, list of bug subscribers is confusing"
  https://bugs.launchpad.net/launchpad/+bug/772754

For more details, see:
https://code.launchpad.net/~danilo/launchpad/bug-772754-other-subscribers-actions/+merge/64187

= Bug 772754: Other subscribers list, part 6 =

NOT READY FOR REVIEW YET

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.  Tests missing.

== Tests ==

lp/bugs/javascript/tests/test_subscribers_list.html

== 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/javascript/bugtask_index_portlets.js
  lib/lp/bugs/javascript/subscribers_list.js
  lib/lp/bugs/templates/bugtask-index.pt
-- 
https://code.launchpad.net/~danilo/launchpad/bug-772754-other-subscribers-actions/+merge/64187
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~danilo/launchpad/bug-772754-other-subscribers-actions into lp:launchpad.

=== modified file 'lib/lp/bugs/browser/bugsubscription.py'
--- lib/lp/bugs/browser/bugsubscription.py	2011-06-10 13:58:29 +0000
+++ lib/lp/bugs/browser/bugsubscription.py	2011-06-10 13:58:31 +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/javascript/bugtask_index_portlets.js'
--- lib/lp/bugs/javascript/bugtask_index_portlets.js	2011-06-10 13:58:29 +0000
+++ lib/lp/bugs/javascript/bugtask_index_portlets.js	2011-06-10 13:58:31 +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-10 13:58:29 +0000
+++ lib/lp/bugs/javascript/subscribers_list.js	2011-06-10 13:58:31 +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
@@ -135,6 +151,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(
@@ -142,6 +159,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 " +
@@ -163,6 +181,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;
 
@@ -266,21 +290,137 @@
  * 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() {
+            subscribers_list.stopSubscriberActivity(subscriber, false);
+        }
+
+        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
+    };
+    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() {
+                    // Failed to get person/team details, thus
+                    // they have not been subscribed.
+                }
+            } });
+    };
+    var 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.
+ */
+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() {
+        loader.subscribers_list.stopSubscriberActivity(
+            subscriber, false, function() {
+                loader.subscribers_list.removeSubscriber(subscriber);
+            }
+        );
+    }
+    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) {
+    if (Y.Lang.isString(LP.links.me) && team.is_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());
+            }
+        }
+
+        var config = {
+            on: { success: on_success }
+        };
+
+        var members_link = team.members_details_collection_link;
+        this.lp_client.get(members_link, config);
+    }
+}
+
 
 /**
  * Manages entire subscribers' list for a single bug.
@@ -893,4 +1033,5 @@
 };
 
 
-}, "0.1", {"requires": ["node", "lazr.anim", "lp.client", "lp.names"]});
+}, "0.1", {"requires": ["node", "lazr.anim", "lp.app.picker", "lp.client",
+                        "lp.names"]});

=== modified file 'lib/lp/bugs/templates/bugtask-index.pt'
--- lib/lp/bugs/templates/bugtask-index.pt	2011-06-10 13:58:29 +0000
+++ lib/lp/bugs/templates/bugtask-index.pt	2011-06-10 13:58:31 +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'
                 });
             });
          });