launchpad-reviewers team mailing list archive

[Brian Murray <brian@xxxxxxxxxx>]

Brian Murray has proposed merging lp:~brian-murray/launchpad/bug-912137 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #912137 in Launchpad itself: "bug subscribers portlet sorted in reverse order"
  https://bugs.launchpad.net/launchpad/+bug/912137

For more details, see:
https://code.launchpad.net/~brian-murray/launchpad/bug-912137/+merge/109045

Summary

If you look at a bug like bug 764701 you'll notice the subscribers are sorted in a reverse order of what you would expect.  This makes it a bit harder to determine whether or not a specific team or person is subscribed to any bug report.  This was reported in bug 912137.

Proposed fix

Instead of inserting subscribers at the start of the list, in subscribers_list.js, we should add them at the end of it.

Tests

bin/test -vv -t test_subscribers_list

Demo and Q/A

I also tested this using the sample data and viewing bug #9 which has two subscribers and confirmed that they were in a normal order.

-- 
https://code.launchpad.net/~brian-murray/launchpad/bug-912137/+merge/109045
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~brian-murray/launchpad/bug-912137 into lp:launchpad.

=== modified file 'lib/lp/app/javascript/subscribers/subscribers_list.js'
--- lib/lp/app/javascript/subscribers/subscribers_list.js	2011-11-18 16:20:50 +0000
+++ lib/lp/app/javascript/subscribers/subscribers_list.js	2012-06-06 22:24:26 +0000
@@ -983,7 +983,7 @@
         subscriber_node = this._createSubscriberNode(subscriber);
         subscriber_node.set('id', subscriber_id);
         // Insert the subscriber at the start of the list.
-        list_node.prepend(subscriber_node);
+        list_node.append(subscriber_node);
         // Add the unsubscribe action if needed.
         if (Y.Lang.isValue(config) &&
             Y.Lang.isFunction(config.unsubscribe_callback)) {

=== modified file 'lib/lp/app/javascript/subscribers/tests/test_subscribers_list.js'
--- lib/lp/app/javascript/subscribers/tests/test_subscribers_list.js	2012-04-06 17:28:25 +0000
+++ lib/lp/app/javascript/subscribers/tests/test_subscribers_list.js	2012-06-06 22:24:26 +0000
@@ -1015,7 +1015,7 @@
 
         test_addSubscriber_ordering: function() {
             // With multiple subscribers being added to the same section,
-            // the last one is listed first.
+            // the last one is listed last.
             var subscribers_list = setUpSubscribersList(this.root);
             var node1 = subscribers_list.addSubscriber(
                 { name: 'user1' }, 'Level3');
@@ -1032,7 +1032,7 @@
                 returned_nodes.push(all_subscribers.item(index));
             }
             Y.ArrayAssert.itemsAreSame(
-                [node2, node1],
+                [node1, node2],
                 returned_nodes);
         },