Merge lp:~diegosarmentero/ubuntuone-control-panel/transfers into lp:ubuntuone-control-panel

[Diego Sarmentero]

- Show current and recent transfers on the transfers menu (LP: #1034542).

You will need this branch:
lp:~diegosarmentero/ubuntuone-client/menu-progress

to test it IRL on windows.

[lp:~diegosarmentero/ubuntuone-control-panel/transfers]

347. By Diego Sarmentero on 2012-08-16

fixing tests

348. By Diego Sarmentero on 2012-08-16

fixed import

349. By Diego Sarmentero on 2012-08-16

improves in progress actoin

[Roberto Alsina (ralsina)]

Looks great!

review: Approve

[Manuel de la Peña (mandel)]

Maybe an assert for the following is better than just a simple comment:

127 + # This is never executed on Ubuntu

and

134 + # This is never executed on Ubuntu

If not an assert a log call.

Also, why do we do the following:

340 + self.patch(systray.TransfersMenu, "startTimer", lambda s, x: None)
341 + yield super(TransfersMenuTestCase, self).setUp()

Do we have to patch it before calling the super version? Can we add a comment with the reason.

The definition of a call like:

384 + def fake_sync_menu():
385 + """Fake backend sync_menu."""
386 + # Return copy of in_progress
387 + return {RECENTTRANSFERS: [], UPLOADING: in_progress[:]}

is done several times. We can do that in the setUp, for example like this:

self.recent_transfers = []
self.uploading = []

def fake_sync_menu():
    return return {RECENTTRANSFERS: self.recent_transfers, UPLOADING: self.uploading}

and just change the value of the vars according to the tests.

review: Needs Fixing

[lp:~diegosarmentero/ubuntuone-control-panel/transfers]

350. By Diego Sarmentero on 2012-08-20

Some improves in logs and tests

[Diego Sarmentero (diegosarmentero)]

> Maybe an assert for the following is better than just a simple comment:
>
> 127 + # This is never executed on Ubuntu
>
> and
>
> 134 + # This is never executed on Ubuntu
>
> If not an assert a log call.
>
> Also, why do we do the following:
>
> 340 + self.patch(systray.TransfersMenu, "startTimer", lambda s, x: None)
> 341 + yield super(TransfersMenuTestCase, self).setUp()
>
> Do we have to patch it before calling the super version? Can we add a comment
> with the reason.
>
> The definition of a call like:
>
> 384 + def fake_sync_menu():
> 385 + """Fake backend sync_menu."""
> 386 + # Return copy of in_progress
> 387 + return {RECENTTRANSFERS: [], UPLOADING: in_progress[:]}
>
> is done several times. We can do that in the setUp, for example like this:
>
> self.recent_transfers = []
> self.uploading = []
>
> def fake_sync_menu():
> return return {RECENTTRANSFERS: self.recent_transfers, UPLOADING:
> self.uploading}
>
> and just change the value of the vars according to the tests.

Done!

[Manuel de la Peña (mandel)]

review: Approve

=== modified file 'ubuntuone/controlpanel/gui/__init__.py'
--- ubuntuone/controlpanel/gui/__init__.py	2012-08-08 18:36:45 +0000
+++ ubuntuone/controlpanel/gui/__init__.py	2012-08-20 12:54:22 +0000
@@ -42,6 +42,10 @@
 SHARES_MIN_SIZE_FULL = 1048576
 SUCCESS_COLOR = u'green'
 
+# Sync Menu:
+RECENTTRANSFERS = 'recent-transfers'
+UPLOADING = 'uploading'
+
 ERROR_ICON = u'✘'
 SYNCING_ICON = u'⇅'
 IDLE_ICON = u'✔'
@@ -179,6 +183,7 @@
 GREETING = _('Hi %(user_display_name)s')
 GO_TO_WEB = _('Go to the Website')
 IN_PROGRESS = _('In Progress')
+IN_PROGRESS_FILE = u'%s – %d%%'
 INSTALL = _('Install')
 INSTALL_PACKAGE = _('You need to install the package <i>%(package_name)s'
                     '</i> in order to enable more sync services.')

=== modified file 'ubuntuone/controlpanel/gui/qt/systray.py'
--- ubuntuone/controlpanel/gui/qt/systray.py	2012-08-13 17:06:15 +0000
+++ ubuntuone/controlpanel/gui/qt/systray.py	2012-08-20 12:54:22 +0000
@@ -15,6 +15,10 @@
 # with this program.  If not, see <http://www.gnu.org/licenses/>.
 """System notification area icon."""
 
+import os
+import sys
+from operator import itemgetter
+
 from PyQt4 import QtGui, QtCore
 from twisted.internet.defer import inlineCallbacks
 from ubuntuone.platform.tools import SyncDaemonTool
@@ -28,10 +32,16 @@
     GET_STORAGE_LINK,
     GET_SUPPORT_LINK,
     GO_TO_WEB,
+    IN_PROGRESS,
+    IN_PROGRESS_FILE,
     LOADING,
     OPEN_UBUNTU_ONE,
     OPEN_UBUNTU_ONE_FOLDER,
     PLEASE_WAIT,
+    RECENT_TRANSFERS,
+    RECENTTRANSFERS,
+    TRANSFERS,
+    UPLOADING,
 )
 from ubuntuone.controlpanel.gui.qt import (
     FILE_SYNC_STATUS,
@@ -42,6 +52,37 @@
 logger = setup_logging('qt.systray')
 
 
+class ProgressBarAction(QtGui.QWidgetAction):
+
+    """Menu action that display a progress bar for the uploads."""
+
+    def __init__(self, filename, percentage, parent=None):
+        super(ProgressBarAction, self).__init__(parent)
+        self._filename = os.path.basename(filename)
+        self.widget = QtGui.QWidget(None)
+        text = IN_PROGRESS_FILE % (self._filename, percentage)
+        self.setText(text)
+        self.label = QtGui.QLabel(text)
+        self.progress = QtGui.QProgressBar()
+        self.progress.setTextVisible(False)
+        self.progress.setValue(percentage)
+        self.progress.setFixedHeight(10)
+        vbox = QtGui.QVBoxLayout()
+        vbox.setContentsMargins(10, 5, 10, 5)
+        vbox.addWidget(self.label)
+        vbox.addWidget(self.progress)
+        self.widget.setLayout(vbox)
+
+        self.setDefaultWidget(self.widget)
+
+    def set_value(self, progress):
+        """Set the value of the progress bar."""
+        text = IN_PROGRESS_FILE % (self._filename, progress)
+        self.setText(text)
+        self.label.setText(text)
+        self.progress.setValue(progress)
+
+
 class TrayIcon(QtGui.QSystemTrayIcon, cache.Cache):
 
     """System notification icon."""
@@ -74,7 +115,9 @@
 
         # TODO: Shares
 
-        # TODO: Transfers
+        # Transfers
+        self.transfers = TransfersMenu(self)
+        self.context_menu.addMenu(self.transfers)
 
         self.get_more_storage = self.context_menu.addAction(GET_MORE_STORAGE)
         self.get_help_online = self.context_menu.addAction(GET_HELP_ONLINE)
@@ -192,3 +235,80 @@
             # Maybe it was not running?
             pass
         self.close_callback()
+
+
+class TransfersMenu(QtGui.QMenu):
+
+    """Menu that shows recent and current transfers."""
+
+    def __init__(self, parent):
+        super(TransfersMenu, self).__init__(TRANSFERS)
+        self._parent = parent
+        self.uploading = {}
+        self.previous_data = {}
+
+        if sys.platform not in ('win32', 'darwin'):
+            self._timer_id = self.startTimer(1000)
+
+    # pylint: disable=C0103
+
+    def showEvent(self, event):
+        """Executed when the transfers menu is shown."""
+        logger.info('This is never executed on Ubuntu')
+        super(TransfersMenu, self).showEvent(event)
+        self.get_transfers_data()
+        self._timer_id = self.startTimer(1000)
+
+    def hideEvent(self, event):
+        """Executed when the transfers menu is hidden."""
+        logger.info('This is never executed on Ubuntu')
+        super(TransfersMenu, self).hideEvent(event)
+        self.killTimer(self._timer_id)
+
+    def timerEvent(self, event):
+        """Update the menu on each iteration."""
+        self.get_transfers_data()
+
+    # pylint: enable=C0103
+
+    @inlineCallbacks
+    def get_transfers_data(self):
+        """Get the transfers data to create the submenu."""
+        data = yield self._parent.backend.sync_menu()
+        self._update_transfers(data)
+
+    def _update_transfers(self, data):
+        """Generate the transfers menu."""
+        current_transfers = data[RECENTTRANSFERS]
+        current_upload = [
+            filename for filename, _, _ in data[UPLOADING]]
+        uploading_data = data[UPLOADING]
+        uploading_data.sort(key=itemgetter(2))
+        uploading_data.reverse()
+        if current_transfers != self.previous_data.get('transfers') or \
+           current_upload != self.previous_data.get('uploading'):
+            self.clear()
+            self.previous_data['transfers'] = current_transfers
+            self.previous_data['uploading'] = current_upload
+            self.uploading.clear()
+            recent = self.addAction(RECENT_TRANSFERS)
+            recent.setEnabled(False)
+            for filename in data[RECENTTRANSFERS]:
+                self.addAction(os.path.basename(filename))
+            self.addSeparator()
+            in_progress = self.addAction(IN_PROGRESS)
+            in_progress.setEnabled(False)
+            show_max = 0
+            for filename, size, written in uploading_data:
+                if show_max == 5:
+                    break
+                percentage = written * 100 / size
+                self.uploading[filename] = ProgressBarAction(
+                    filename, percentage)
+                self.addAction(self.uploading[filename])
+                show_max += 1
+        else:
+            for filename, size, written in uploading_data:
+                percentage = written * 100 / size
+                if filename in self.uploading:
+                    self.uploading[filename].set_value(percentage)

=== modified file 'ubuntuone/controlpanel/gui/qt/tests/__init__.py'
--- ubuntuone/controlpanel/gui/qt/tests/__init__.py	2012-04-02 11:10:49 +0000
+++ ubuntuone/controlpanel/gui/qt/tests/__init__.py	2012-08-20 12:54:22 +0000
@@ -25,7 +25,11 @@
 
 from ubuntuone.controlpanel import backend, cache
 from ubuntuone.controlpanel.tests import TestCase, EXPECTED_ACCOUNT_INFO, TOKEN
-from ubuntuone.controlpanel.gui import qt
+from ubuntuone.controlpanel.gui import (
+    qt,
+    RECENTTRANSFERS,
+    UPLOADING,
+)
 from ubuntuone.controlpanel.gui.tests import (
     FakedObject,
     FAKE_VOLUMES_INFO,
@@ -164,6 +168,13 @@
     def build_signed_iri(self, iri):
         """Fake iri signing."""
 
+    def sync_menu(self):
+        """Fake sync_menu."""
+        data = {}
+        data[RECENTTRANSFERS] = []
+        data[UPLOADING] = []
+        return data
+
 
 class CrashyBackendException(Exception):
     """A faked backend crash."""

=== modified file 'ubuntuone/controlpanel/gui/qt/tests/test_systray.py'
--- ubuntuone/controlpanel/gui/qt/tests/test_systray.py	2012-08-14 18:55:56 +0000
+++ ubuntuone/controlpanel/gui/qt/tests/test_systray.py	2012-08-20 12:54:22 +0000
@@ -22,7 +22,14 @@
 from twisted.internet.defer import inlineCallbacks
 
 import ubuntuone.controlpanel.gui.qt.gui
-from ubuntuone.controlpanel.gui import qt
+from ubuntuone.controlpanel.gui import (
+    qt,
+    IN_PROGRESS,
+    IN_PROGRESS_FILE,
+    RECENT_TRANSFERS,
+    RECENTTRANSFERS,
+    UPLOADING,
+)
 from ubuntuone.controlpanel.gui.qt import systray
 from ubuntuone.controlpanel.gui.qt.tests import BaseTestCase
 from ubuntuone.controlpanel.tests import ROOT_PATH
@@ -73,6 +80,7 @@
 
     @inlineCallbacks
     def setUp(self):
+        self.patch(systray.TransfersMenu, "startTimer", lambda s, x: None)
         yield super(SystrayTestCase, self).setUp()
         self.patch(QtGui, "QDesktopServices", FakeDesktopService)
 
@@ -194,6 +202,7 @@
         """Test the quit option in the menu."""
         # Not done on setup, because if I patch stop
         # after instantiation, it doesn't get called.
+        self.patch(systray.TransfersMenu, "startTimer", lambda s, x: None)
         self.patch(systray.TrayIcon, "stop", self._set_called)
         tray = systray.TrayIcon()
         tray.quit.trigger()
@@ -203,6 +212,7 @@
     def test_stop_sd(self):
         """Quit should call SyncDaemonTool.quit()."""
         st = FakeSDTool()
+        self.patch(systray.TransfersMenu, "startTimer", lambda s, x: None)
         self.patch(systray, "SyncDaemonTool", lambda: st)
         tray = systray.TrayIcon()
         yield tray.stop()
@@ -210,6 +220,7 @@
 
     def test_restore_no_window(self):
         """Test the restore window option in the menu, with no window."""
+        self.patch(systray.TransfersMenu, "startTimer", lambda s, x: None)
         self.patch(ubuntuone.controlpanel.gui.qt.gui,
             "MainWindow", FakeMainWindow)
         tray = systray.TrayIcon()
@@ -222,6 +233,7 @@
 
     def test_restore_window(self):
         """Test the restore window option in the menu, with a window."""
+        self.patch(systray.TransfersMenu, "startTimer", lambda s, x: None)
         tray = systray.TrayIcon()
         window = FakeMainWindow()
         tray.window = window
@@ -232,6 +244,7 @@
 
     def test_restore_window_minimized(self):
         """Test the restore window option in the menu, with a min. window."""
+        self.patch(systray.TransfersMenu, "startTimer", lambda s, x: None)
         tray = systray.TrayIcon()
         window = FakeMainWindow()
         # This cannot be tested with the real activateWindow
@@ -244,6 +257,7 @@
 
     def test_delete_window(self):
         """Test deleting an existing window."""
+        self.patch(systray.TransfersMenu, "startTimer", lambda s, x: None)
         tray = systray.TrayIcon()
         window = FakeMainWindow()
         tray.window = window
@@ -253,6 +267,7 @@
 
     def test_delete_no_window(self):
         """Test deleting without an existing window."""
+        self.patch(systray.TransfersMenu, "startTimer", lambda s, x: None)
         tray = systray.TrayIcon()
         tray.delete_window()
         self.assertEqual(tray.window, None)
@@ -287,6 +302,7 @@
 
     def test_initialization(self):
         """Test that everything initializes correctly."""
+        self.patch(systray.TransfersMenu, "startTimer", lambda s, x: None)
         tray = systray.TrayIcon()
         self.assertTrue(tray.isVisible())
         self.assertEqual(tray.window, None)
@@ -301,7 +317,223 @@
         self.assertIsInstance(tray.go_to_web, QtGui.QAction)
         self.assertIsInstance(tray.get_more_storage, QtGui.QAction)
         self.assertIsInstance(tray.get_help_online, QtGui.QAction)
+        self.assertIsInstance(tray.transfers, QtGui.QMenu)
         # This checks that _get_volumes_info and _process_volumes_info
         # is being called correctly on initialization.
         expected_home = u'file://%s' % ROOT_PATH
         self.assertEqual(tray.root_path, expected_home)
+
+    def test_get_transfers_data(self):
+        """Check that _get_transfers_data return the proper data."""
+
+        result = []
+
+        def fake_callback(data):
+            """Fake callback to process data."""
+            result.append(data)
+
+        self.patch(self.ui.transfers, "_update_transfers", fake_callback)
+        self.ui.transfers.get_transfers_data()
+        self.assertEqual(result, [{RECENTTRANSFERS: [], UPLOADING: []}])
+
+
+class TransfersMenuTestCase(BaseTestCase):
+
+    """Test the notification area icon."""
+
+    class_ui = systray.TrayIcon
+
+    @inlineCallbacks
+    def setUp(self):
+        self.patch(systray.TransfersMenu, "startTimer", lambda s, x: None)
+        yield super(TransfersMenuTestCase, self).setUp()
+        self.patch(QtGui, "QDesktopServices", FakeDesktopService)
+        self.patch(self.ui.transfers._parent.backend, "sync_menu",
+            self.fake_sync_menu)
+        self.recent_transfers = []
+        self.uploading = []
+
+    def fake_sync_menu(self):
+        """Fake backend sync_menu."""
+        return {RECENTTRANSFERS: self.recent_transfers,
+                UPLOADING: self.uploading}
+
+    def test_load_menu(self):
+        """Show the menu with just the labels."""
+        self.ui.transfers.get_transfers_data()
+        actions = self.ui.transfers.actions()
+        self.assertEqual(actions[0].text(), RECENT_TRANSFERS)
+        self.assertTrue(actions[1].isSeparator())
+        self.assertEqual(actions[2].text(), IN_PROGRESS)
+
+    def test_load_recent_transfers(self):
+        """Show the menu with the recent transfers."""
+        self.recent_transfers = ['file1', 'file2']
+        self.ui.transfers.get_transfers_data()
+        actions = self.ui.transfers.actions()
+        self.assertEqual(actions[0].text(), RECENT_TRANSFERS)
+        self.assertEqual(actions[1].text(), 'file1')
+        self.assertEqual(actions[2].text(), 'file2')
+        self.assertTrue(actions[3].isSeparator())
+        self.assertEqual(actions[4].text(), IN_PROGRESS)
+
+    def test_load_in_progress(self):
+        """Show the menu with the current progress."""
+        in_progress = [
+            ('file1', 200, 150),
+            ('file2', 400, 100),
+            ('file3', 300, 200),
+        ]
+
+        # Return copy of in_progress
+        self.uploading = in_progress[:]
+        self.ui.transfers.get_transfers_data()
+        actions = self.ui.transfers.actions()
+        self.assertEqual(actions[0].text(), RECENT_TRANSFERS)
+        self.assertTrue(actions[1].isSeparator())
+        self.assertEqual(actions[2].text(), IN_PROGRESS)
+
+        # This also check that the files are ordered based on written
+        percentage = in_progress[2][2] * 100 / in_progress[2][1]
+        text = IN_PROGRESS_FILE % (in_progress[2][0], percentage)
+        self.assertEqual(actions[3].label.text(), text)
+        self.assertEqual(actions[3].progress.value(), percentage)
+
+        percentage = in_progress[0][2] * 100 / in_progress[0][1]
+        text = IN_PROGRESS_FILE % (in_progress[0][0], percentage)
+        self.assertEqual(actions[4].label.text(), text)
+        self.assertEqual(actions[4].progress.value(), percentage)
+
+        percentage = in_progress[1][2] * 100 / in_progress[1][1]
+        text = IN_PROGRESS_FILE % (in_progress[1][0], percentage)
+        self.assertEqual(actions[5].label.text(), text)
+        self.assertEqual(actions[5].progress.value(), percentage)
+
+    def test_load_in_progress_refresh(self):
+        """Show the menu with the current progress and refresh it."""
+        in_progress = [
+            ('file1', 200, 150),
+            ('file2', 400, 100),
+            ('file3', 300, 200),
+        ]
+        # Return copy of in_progress
+        self.uploading = in_progress[:]
+        self.ui.transfers.get_transfers_data()
+        actions = self.ui.transfers.actions()
+        self.assertEqual(actions[0].text(), RECENT_TRANSFERS)
+        self.assertTrue(actions[1].isSeparator())
+        self.assertEqual(actions[2].text(), IN_PROGRESS)
+
+        # This also check that the files are ordered based on written
+        previous_actions = []
+        percentage = in_progress[2][2] * 100 / in_progress[2][1]
+        text = IN_PROGRESS_FILE % (in_progress[2][0], percentage)
+        self.assertEqual(actions[3].text(), text)
+        previous_actions.append(actions[3])
+
+        percentage = in_progress[0][2] * 100 / in_progress[0][1]
+        text = IN_PROGRESS_FILE % (in_progress[0][0], percentage)
+        self.assertEqual(actions[4].text(), text)
+        previous_actions.append(actions[4])
+
+        percentage = in_progress[1][2] * 100 / in_progress[1][1]
+        text = IN_PROGRESS_FILE % (in_progress[1][0], percentage)
+        self.assertEqual(actions[5].text(), text)
+        previous_actions.append(actions[5])
+
+        in_progress = [
+            ('file1', 200, 170),
+            ('file2', 400, 300),
+            ('file3', 300, 210),
+        ]
+        self.uploading = in_progress[:]
+
+        self.ui.transfers.get_transfers_data()
+        actions = self.ui.transfers.actions()
+        current_actions = []
+        self.assertEqual(actions[0].text(), RECENT_TRANSFERS)
+        self.assertTrue(actions[1].isSeparator())
+        self.assertEqual(actions[2].text(), IN_PROGRESS)
+
+        # This also check that the files are ordered based on written
+        percentage = in_progress[2][2] * 100 / in_progress[2][1]
+        text = IN_PROGRESS_FILE % (in_progress[2][0], percentage)
+        self.assertEqual(actions[3].text(), text)
+        current_actions.append(actions[3])
+
+        percentage = in_progress[0][2] * 100 / in_progress[0][1]
+        text = IN_PROGRESS_FILE % (in_progress[0][0], percentage)
+        self.assertEqual(actions[4].text(), text)
+        current_actions.append(actions[4])
+
+        percentage = in_progress[1][2] * 100 / in_progress[1][1]
+        text = IN_PROGRESS_FILE % (in_progress[1][0], percentage)
+        self.assertEqual(actions[5].text(), text)
+        current_actions.append(actions[5])
+
+        self.assertEqual(previous_actions, current_actions)
+
+    def test_menu_not_reload(self):
+        """Show the menu and test that is not reload it on refresh."""
+        self.recent_transfers = ['file1', 'file2']
+        self.ui.transfers.get_transfers_data()
+        previous_actions = self.ui.transfers.actions()
+        self.ui.transfers.get_transfers_data()
+        current_actions = self.ui.transfers.actions()
+        self.assertEqual(previous_actions, current_actions)
+
+    def test_menu_reload(self):
+        """Show the menu and test that is reload it on refresh."""
+        self.recent_transfers = ['file1', 'file2']
+        self.ui.transfers.get_transfers_data()
+        previous_actions = self.ui.transfers.actions()
+        self.recent_transfers = ['file1', 'file2', 'file3']
+
+        self.ui.transfers.get_transfers_data()
+        current_actions = self.ui.transfers.actions()
+        self.assertNotEqual(previous_actions, current_actions)
+
+    def test_progress_not_reload(self):
+        """Show the menu and test that is not reload it on refresh."""
+        in_progress = [
+            ('file1', 200, 150),
+            ('file2', 400, 100),
+            ('file3', 300, 200),
+        ]
+        # Return copy of in_progress
+        self.uploading = in_progress[:]
+        self.ui.transfers.get_transfers_data()
+        previous_actions = self.ui.transfers.actions()
+        in_progress = [
+            ('file1', 200, 170),
+            ('file2', 400, 300),
+            ('file3', 300, 210),
+        ]
+        self.uploading = in_progress[:]
+        self.ui.transfers.get_transfers_data()
+        current_actions = self.ui.transfers.actions()
+
+        self.assertEqual(previous_actions, current_actions)
+
+    def test_progress_reload(self):
+        """Show the menu and test that is reload it on refresh."""
+        in_progress = [
+            ('file1', 200, 150),
+            ('file2', 400, 100),
+            ('file3', 300, 200),
+        ]
+        # Return copy of in_progress
+        self.uploading = in_progress[:]
+        self.ui.transfers.get_transfers_data()
+        previous_actions = self.ui.transfers.actions()
+        in_progress = [
+            ('file1', 200, 170),
+            ('file2', 400, 300),
+            ('file3', 300, 210),
+            ('file4', 1000, 410),
+        ]
+        self.uploading = in_progress[:]
+        self.ui.transfers.get_transfers_data()
+        current_actions = self.ui.transfers.actions()
+
+        self.assertNotEqual(previous_actions, current_actions)