Merge lp:~diegosarmentero/ubuntuone-client/ubuntuone-client-menuorder into lp:ubuntuone-client

[Diego Sarmentero]

- Sort items based on the written value, to prioritize the files being transferred (LP: #1052956).

[dobey (dobey)]

The implications of how this might behave, frighten me a little. A video showing this in action in an open transfers menu, which remains open for many (at least 30) seconds, would be quite helpful.

But based on the bug report, and that this code will presumably be updated many times while the menu is being examined by a user, it seems like the ordering may change. And having menu items constantly changing position would be quite disturbing for the user; even if the menu items are not clickable (as they do not seem to be).

review: Needs Information

[lp:~diegosarmentero/ubuntuone-client/ubuntuone-client-menuorder]

1325. By Diego Sarmentero on 2012-09-21

merge

[Diego Sarmentero (diegosarmentero)]

> The implications of how this might behave, frighten me a little. A video
> showing this in action in an open transfers menu, which remains open for many
> (at least 30) seconds, would be quite helpful.
>
> But based on the bug report, and that this code will presumably be updated
> many times while the menu is being examined by a user, it seems like the
> ordering may change. And having menu items constantly changing position would
> be quite disturbing for the user; even if the menu items are not clickable (as
> they do not seem to be).

This is the video i showed you on irc: http://youtu.be/qOmaBCayQAo

Take into account that i'm not deleting all the items and recreating the menu with the ones that has the highest written value, but just deleting the ones that complete the transfer, and always prioritizing the ones with the highest written value to put in those places.

[dobey (dobey)]

review: Approve

[lp:~diegosarmentero/ubuntuone-client/ubuntuone-client-menuorder]

1326. By Diego Sarmentero on 2012-09-25

fixing tests

1327. By Diego Sarmentero on 2012-09-25

tests updated

1328. By Diego Sarmentero on 2012-09-25

merge

1329. By Diego Sarmentero on 2012-09-26

merge

1330. By Diego Sarmentero on 2012-09-26

merge

[Manuel de la Peña (mandel)]

I really like the use of itemgetter. +1

review: Approve

=== modified file 'tests/platform/sync_menu/test_linux.py'
--- tests/platform/sync_menu/test_linux.py	2012-09-26 20:24:20 +0000
+++ tests/platform/sync_menu/test_linux.py	2012-09-26 20:24:20 +0000
@@ -343,13 +343,43 @@
                 linux.SyncMenu.PROGRESS_MENUITEM_PROP_PERCENT_DONE),
                 percentage)
 
+    def test_transfers_order(self):
+        """Check that the proper transfers are shown first."""
+        data_current = [
+            ('file0', 1200, 610),
+            ('file1', 3000, 400),
+            ('file2', 2000, 100),
+            ('file3', 2500, 150),
+            ('file4', 2500, 950),
+            ('file5', 3500, 550),
+            ('file6', 1000, 600),
+            ('file7', 5000, 4600)]
+        expected = [
+            ('file7', 5000, 4600),
+            ('file4', 2500, 950),
+            ('file0', 1200, 610),
+            ('file6', 1000, 600),
+            ('file5', 3500, 550)]
+        self.status_frontend.uploading_data = data_current
+        self.sync_menu.transfers.update_progress()
+        children = self.sync_menu.transfers.get_children()
+        # The menu should only show 5 current transfers.
+        self.assertEqual(len(children), 5)
+        for i, item in enumerate(children):
+            text = item.property_get(linux.Dbusmenu.MENUITEM_PROP_LABEL)
+            percentage = item.property_get_int(
+                linux.SyncMenu.PROGRESS_MENUITEM_PROP_PERCENT_DONE)
+            name, size, written = expected[i]
+            percentage_expected = written * 100 / size
+            self.assertEqual(text, name)
+            self.assertEqual(percentage, percentage_expected)
+
     def test_update_transfers_delay(self):
         """Check that the timer is being handle properly."""
         self.patch(linux.status.aggregator, "Timer", FakeTimer)
-        self.sync_menu.next_update = time.time() + 3
+        self.sync_menu.next_update = time.time()
         self.sync_menu.update_transfers()
-        self.assertLess(self.sync_menu.timer.delay, 3)
         self.sync_menu.timer = None
-        self.sync_menu.next_update = time.time() / 2
+        self.sync_menu.next_update = time.time() * 2
         self.sync_menu.update_transfers()
-        self.assertEqual(self.sync_menu.timer.delay, 0)
+        self.assertEqual(self.sync_menu.timer.delay, 3)

=== modified file 'ubuntuone/platform/sync_menu/linux.py'
--- ubuntuone/platform/sync_menu/linux.py	2012-09-26 20:24:20 +0000
+++ ubuntuone/platform/sync_menu/linux.py	2012-09-26 20:24:20 +0000
@@ -33,6 +33,8 @@
 import time
 import sys
 import webbrowser
+from collections import OrderedDict
+from operator import itemgetter
 
 glib = None
 try:
@@ -182,18 +184,20 @@
 
     def update_progress(self):
         """Update the list of recent transfers and current transfers."""
-        current_transfers = self.status_frontend.recent_transfers()
-        uploading_data = {}
-        for filename, size, written in \
-            self.status_frontend.files_uploading():
+        recent_transfers = self.status_frontend.recent_transfers()
+        current_transfers = self.status_frontend.files_uploading()
+        current_transfers.sort(key=itemgetter(2))
+        current_transfers.reverse()
+        uploading_data = OrderedDict()
+        for filename, size, written in current_transfers:
             uploading_data[filename] = (size, written)
 
         temp_transfers = {}
-        if current_transfers != self.previous_transfers:
-            logger.debug("Update recent transfers with: %r", current_transfers)
+        if recent_transfers != self.previous_transfers:
+            logger.debug("Update recent transfers with: %r", recent_transfers)
             for item_transfer in self._transfers_items:
                 self.child_delete(self._transfers_items[item_transfer])
-            for item in current_transfers:
+            for item in recent_transfers:
                 recent_file = Dbusmenu.Menuitem()
                 recent_file.property_set(Dbusmenu.MENUITEM_PROP_LABEL,
                     item)