launchpad-reviewers team mailing list archive

[Ian Booth <ian.booth@xxxxxxxxxxxxx>]

Ian Booth has proposed merging lp:~wallyworld/launchpad/update-bug-subscribers-portlet-850500 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #850500 in Launchpad itself: "Update bug subscribers portlet after making bug private"
  https://bugs.launchpad.net/launchpad/+bug/850500

For more details, see:
https://code.launchpad.net/~wallyworld/launchpad/update-bug-subscribers-portlet-850500/+merge/75481

See bug 850500.
When the lp ui is used to mark a bug private via ajax, the subscription portlet is not automatically updated to show the changed subscribers list. A page refresh is required. The portlet should be automatically updated. 

== Implementation ==

Simply extend the BugSecrecyEditView`to stuff the required subscribers data into the json data that it returns. The subscribers data comes directly from rendering the BugPortletSubscribersWithDetails() view.
The bugtask_index javascript has been tweaked to take the subscribers data and invoke the _loadSubscribersFromList() API. It also invokes start/stopActivity() to indicate that the subscribers list is being updated.

== Tests ==

Modify the existing test_secrecy_view_ajax_render test to account for the new data returned by the view.

== Lint==

Linting changed files:
  lib/lp/app/javascript/subscribers/subscribers_list.js
  lib/lp/bugs/browser/bug.py
  lib/lp/bugs/browser/tests/test_bug_views.py
  lib/lp/bugs/javascript/bugtask_index.js


-- 
https://code.launchpad.net/~wallyworld/launchpad/update-bug-subscribers-portlet-850500/+merge/75481
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wallyworld/launchpad/update-bug-subscribers-portlet-850500 into lp:launchpad.

=== modified file 'lib/lp/app/javascript/subscribers/subscribers_list.js'
--- lib/lp/app/javascript/subscribers/subscribers_list.js	2011-08-30 12:41:13 +0000
+++ lib/lp/app/javascript/subscribers/subscribers_list.js	2011-09-15 05:37:02 +0000
@@ -85,6 +85,7 @@
     }
     this.subscriber_levels = config.subscriber_levels;
     var sl = this.subscribers_list = new SubscribersList(config);
+    sl.container_node.setData('subscribers_loader', this);
 
     if (!Y.Lang.isValue(config.context) ||
         !Y.Lang.isString(config.context.web_link)) {
@@ -211,6 +212,7 @@
         Y.error('Got non-array "'+ details +
                 '" in _loadSubscribersFromList().');
     }
+    this.subscribers_list.container_node.get('childNodes').remove();
     var index, subscriber;
     for (index = 0; index < details.length; index++) {
         subscriber = details[index].subscriber;

=== modified file 'lib/lp/bugs/browser/bug.py'
--- lib/lp/bugs/browser/bug.py	2011-09-12 01:27:24 +0000
+++ lib/lp/bugs/browser/bug.py	2011-09-15 05:37:02 +0000
@@ -2,6 +2,7 @@
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """IBug related view classes."""
+from lp.bugs.browser.bugsubscription import BugPortletSubscribersWithDetails
 
 __metaclass__ = type
 
@@ -104,7 +105,7 @@
     BugTaskSearchParams,
     BugTaskStatus,
     IFrontPageBugTaskSearch,
-    )
+    IBugTask)
 from lp.bugs.interfaces.bugwatch import IBugWatchSet
 from lp.bugs.interfaces.cve import ICveSet
 from lp.bugs.mail.bugnotificationbuilder import format_rfc2822_date
@@ -870,18 +871,34 @@
             notify(ObjectModifiedEvent(
                     bug, bug_before_modification, changed_fields))
         if self.request.is_ajax:
-            if private_changed:
+            if private_changed or security_related_changed:
                 return self._getSubscriptionDetails()
             else:
                 return ''
 
     def _getSubscriptionDetails(self):
         cache = dict()
+        # The subscription details for the current user.
         self.extractBugSubscriptionDetails(self.user, self.context.bug, cache)
-        return dumps(
+
+        # The subscription details for other users to populate the subscribers
+        # list in the portlet.
+        if IBugTask.providedBy(self.context):
+            bug = self.context.bug
+        else:
+            bug = self.context
+        subscribers_portlet = BugPortletSubscribersWithDetails(
+            bug, self.request)
+        subscription_data = subscribers_portlet()
+        cache_data = dumps(
             cache, cls=ResourceJSONEncoder,
             media_type=EntryResource.JSON_TYPE)
 
+        return ("""{
+                "cache_data": %s,
+                "subscription_data": %s}
+            """) % (cache_data, subscription_data)
+
     def _handlePrivacyChanged(self, user_will_be_subscribed):
         """Handle the case where the privacy of the bug has been changed.
 

=== modified file 'lib/lp/bugs/browser/tests/test_bug_views.py'
--- lib/lp/bugs/browser/tests/test_bug_views.py	2011-09-12 01:24:44 +0000
+++ lib/lp/bugs/browser/tests/test_bug_views.py	2011-09-15 05:37:02 +0000
@@ -7,6 +7,7 @@
 
 import simplejson
 from zope.component import getUtility
+from zope.security.proxy import removeSecurityProxy
 
 from BeautifulSoup import BeautifulSoup
 
@@ -273,7 +274,8 @@
         # When the bug secrecy view is called from an ajax request, it should
         # provide a json encoded dict when rendered. The dict contains bug
         # subscription information resulting from the update to the bug
-        # privacy.
+        # privacy as well as information used to populate the updated
+        # subscribers list.
         person = self.factory.makePerson()
         bug = self.factory.makeBug(owner=person)
         with person_logged_in(person):
@@ -287,7 +289,9 @@
                 'field.security_related': 'off'},
             **extra)
         view = self.createInitializedSecrecyView(person, bug, request)
-        cache_data = simplejson.loads(view.render())
+        result_data = simplejson.loads(view.render())
+
+        cache_data = result_data['cache_data']
         self.assertFalse(cache_data['other_subscription_notifications'])
         subscription_data = cache_data['subscription']
         self.assertEqual(
@@ -299,6 +303,12 @@
         self.assertEqual(
             'Discussion', subscription_data['bug_notification_level'])
 
+        [subscriber_data] = result_data['subscription_data']
+        subscriber = removeSecurityProxy(bug.default_bugtask).pillar.owner
+        self.assertEqual(
+            subscriber.name, subscriber_data['subscriber']['name'])
+        self.assertEqual('Discussion', subscriber_data['subscription_level'])
+
     def test_set_security_related(self):
         # Test that the bug attribute 'security_related' can be updated
         # using the view.

=== modified file 'lib/lp/bugs/javascript/bugtask_index.js'
--- lib/lp/bugs/javascript/bugtask_index.js	2011-09-14 08:59:04 +0000
+++ lib/lp/bugs/javascript/bugtask_index.js	2011-09-15 05:37:02 +0000
@@ -366,6 +366,10 @@
     qs = Y.lp.client.append_qs(qs, 'field.private', private_flag?'on':'off');
     qs = Y.lp.client.append_qs(
         qs, 'field.security_related', security_related?'on':'off');
+    var sub_list_node = Y.one('#other-bug-subscribers');
+    var subscribers_list = sub_list_node.getData('subscribers_loader');
+    subscribers_list.subscribers_list.startActivity(
+            'Updating subscribers...');
     var config = {
         method: "POST",
         headers: {'Accept': 'application/xhtml'},
@@ -373,7 +377,11 @@
         on: {
             success: function (id, response) {
                 if (response.responseText !== '') {
-                    var cache_data = Y.JSON.parse(response.responseText);
+                    var result_data = Y.JSON.parse(response.responseText);
+                    var subscribers = result_data.subscription_data;
+                    subscribers_list._loadSubscribersFromList(subscribers);
+                    subscribers_list.subscribers_list.stopActivity();
+                    var cache_data = result_data.cache_data;
                     var item;
                     for (item in cache_data) {
                         if (cache_data.hasOwnProperty(item)) {
@@ -382,6 +390,8 @@
                     }
                     var ns = Y.lp.bugs.bugtask_index.portlets.subscription;
                     ns.update_subscription_status();
+                } else {
+                    subscribers_list.subscribers_list.stopActivity();
                 }
                 privacy_spinner.setStyle('display', 'none');
                 privacy_link.setStyle('display', 'inline');
@@ -432,7 +442,11 @@
                         response.getResponseHeader('X-Lazr-Notifications'));
                 Y.lp.anim.green_flash({node: privacy_div}).run();
             },
-            failure: error_handler.getFailureHandler()
+            failure: function (ioId, o) {
+                subscribers_list.subscribers_list.stopActivity();
+                var handler = error_handler.getFailureHandler();
+                handler(ioId, o);
+            }
         }
     };
     lp_client.io_provider.io(submit_url, config);