Merge lp:~ralsina/ubuntuone-control-panel/sort-shares into lp:ubuntuone-control-panel

[Roberto Alsina]

 - Enable folder list sorting (Fixes LP:1006385).

Enable sorting by the 1st column in the control panel's folder list.

This has one maybe unexpected side effect, which is that on languages where "My personal folders" is > "Shared by" you get the shares on top. I don't think that is really a problem.

The sort order is tested by test_process_info, which will compare the item order to case-insensitive-sorted data.

[Manuel de la Peña (mandel)]

review: Approve

[dobey (dobey)]

review: Approve

[Ubuntu One Auto Pilot (otto-pilot)]

The attempt to merge lp:~ralsina/ubuntuone-control-panel/sort-shares into lp:ubuntuone-control-panel failed. Below is the output from the failed tests.

*** Running test suite for ubuntuone/controlpanel ***
ubuntuone.controlpanel.tests.test_replication_client
  ReplicationsTestCase
    test_exclude ... [OK]
    test_exclude_name_in_exclusions ... [OK]
    test_exclude_name_not_in_replications ... [OK]
    test_get_exclusions ... [OK]
    test_get_replications ... [OK]
    test_no_pairing_record ... [OK]
    test_replicate ... [OK]
    test_replicate_name_not_in_exclusions ... [OK]
    test_replicate_name_not_in_replications ... [OK]
ubuntuone.controlpanel.tests
  TestCase
    runTest ... [OK]
ubuntuone.controlpanel.tests.test_sd_client
  AutoconnectTestCase
    test_disable ... [OK]
    test_disable_throws_an_error ... [OK]
    test_enable ... [OK]
    test_enable_throws_an_error ... [OK]
    test_get_value ... [OK]
    test_get_value_throws_an_error ... [OK]
  BaseTestCase
    runTest ... [OK]
  BasicTestCase
    test_get_current_status ... [OK]
    test_get_current_status_error ... [OK]
    test_get_root_dir ... [OK]
    test_get_shares_dir ... [OK]
    test_get_shares_dir_link ... [OK]
  FileSyncTestCase
    test_connect_file_sync ... [OK]
    test_disable_file_sync ... [OK]
    test_disconnect_file_sync ... [OK]
    test_enable_file_sync ... [OK]
    test_file_sync_enabled ... [OK]
    test_set_status_changed_handler ... [OK]
    test_start_file_sync ... [OK]
    test_stop_file_sync ... [OK]
  FoldersTestCase
    test_create_folder ... [OK]
    test_create_folder_error ... [OK]
    test_get_folders ... [OK]
    test_get_folders_error ... [OK]
    test_subscribe_fold...

[lp:~ralsina/ubuntuone-control-panel/sort-shares]

332. By Roberto Alsina on 2012-06-05

make test_focus_order not depend on the order of the items

333. By Roberto Alsina on 2012-06-05

no more tests relying on the order of the items on the display

[dobey (dobey)]

+ #self.assertTrue(item is None)

Why is this commented out?

review: Needs Fixing

[lp:~ralsina/ubuntuone-control-panel/sort-shares]

334. By Roberto Alsina on 2012-06-06

removed remnant

[Roberto Alsina (ralsina)]

> + #self.assertTrue(item is None)
>
> Why is this commented out?

Leftover from old code. Removed in revno 334

[dobey (dobey)]

review: Approve

=== modified file 'ubuntuone/controlpanel/gui/qt/folders.py'
--- ubuntuone/controlpanel/gui/qt/folders.py	2012-05-29 14:30:36 +0000
+++ ubuntuone/controlpanel/gui/qt/folders.py	2012-06-06 15:30:26 +0000
@@ -137,6 +137,9 @@
         headers.setResizeMode(SUBSCRIPTION_COL, headers.ResizeToContents)
         headers.setResizeMode(EXPLORE_COL, headers.ResizeToContents)
 
+        self.ui.folders.setSortingEnabled(True)
+        self.ui.folders.sortByColumn(FOLDER_NAME_COL, QtCore.Qt.AscendingOrder)
+
         self.ui.share_publish_button.setText(FOLDERS_MANAGE_LABEL)
         self.ui.share_publish_button.uri = MANAGE_FILES_LINK
         QtGui.QApplication.instance().focusChanged.connect(self.focus_changed)

=== modified file 'ubuntuone/controlpanel/gui/qt/tests/test_folders.py'
--- ubuntuone/controlpanel/gui/qt/tests/test_folders.py	2012-05-29 17:18:55 +0000
+++ ubuntuone/controlpanel/gui/qt/tests/test_folders.py	2012-06-06 15:30:26 +0000
@@ -24,7 +24,7 @@
 import shutil
 import sys
 
-from PyQt4 import QtGui
+from PyQt4 import QtCore, QtGui
 from twisted.internet import defer
 from ubuntuone.devtools.handlers import MementoHandler
 from ubuntuone.devtools.testcases import skipIfOS
@@ -244,28 +244,21 @@
 
     def test_process_info(self):
         """The volumes info is processed when ready."""
+        folders = self.ui.ui.folders
         self.ui.process_info(FAKE_VOLUMES_INFO)
-        folders = self.ui.ui.folders
-
         root = folders.invisibleRootItem()
         self.assertEqual(len(FAKE_VOLUMES_INFO), root.childCount())
 
-        treeiter = gui.QtGui.QTreeWidgetItemIterator(folders)
         for name, _, volumes in FAKE_VOLUMES_INFO:
             name = _build_name(name)
-
-            # get first child, matching "My Folders"
-            item = treeiter.value()
-            self.assertFalse(item.is_empty)
-
+            items = folders.findItems(name, QtCore.Qt.MatchFixedString
+                | QtCore.Qt.MatchCaseSensitive | QtCore.Qt.MatchRecursive,
+                gui.FOLDER_NAME_COL)
+            self.assertEqual(1, len(items))
+            item = items[0]
             self.assert_folder_group_header_correct(item, name)
-
             self.assertEqual(len(volumes), item.childCount())
-            sorted_vols = sorted(volumes, key=operator.itemgetter('path'))
-            for volume in sorted_vols:
-                treeiter += 1
-                item = treeiter.value()  # get child folder
-                self.assertTrue(item is not None)
+            for volume in volumes:
 
                 name = volume['path'].replace(USER_HOME + os.path.sep, '')
                 expected_path = volume['path']
@@ -278,12 +271,14 @@
                 else:
                     icon_name = gui.SHARE_ICON_NAME
 
+                items = folders.findItems(name, QtCore.Qt.MatchFixedString |
+                    QtCore.Qt.MatchCaseSensitive | QtCore.Qt.MatchRecursive,
+                    gui.FOLDER_NAME_COL)
+                self.assertEqual(1, len(items))
+                item = items[0]
                 self.assert_folder_row_correct(item, name, icon_name, volume,
                                                tweaked_path=expected_path)
 
-            treeiter += 1
-            item = treeiter.value()
-
     def test_process_info_clears_the_list(self):
         """The old volumes info is cleared before updated."""
         self.ui.process_info(FAKE_VOLUMES_INFO)
@@ -305,9 +300,10 @@
 
         child_index = 0
         root = self.ui.ui.folders.invisibleRootItem()
-        for name, _, _ in FAKE_VOLUMES_NO_FREE_SPACE_INFO:
-            name = _build_name(name)
-
+        names = [_build_name(name) for name, _, _ in
+            FAKE_VOLUMES_NO_FREE_SPACE_INFO]
+        names.sort(key=lambda x: x.lower())
+        for name in names:
             item = root.child(child_index)
             self.assertFalse(item.is_empty)
             self.assert_folder_group_header_correct(item, name)
@@ -414,34 +410,23 @@
         """Ensure that the inner widgets are in the correct tab order."""
         self.ui.process_info(FAKE_VOLUMES_INFO)
         folders = self.ui.ui.folders
-
         widget = self.ui.ui.folders.nextInFocusChain()
         treeiter = gui.QtGui.QTreeWidgetItemIterator(folders)
-        for name, _, volumes in FAKE_VOLUMES_INFO:
-            item = treeiter.value()
-            sorted_vols = sorted(volumes, key=operator.itemgetter('path'))
-            for volume in sorted_vols:
+        while treeiter.value():
+            if treeiter.value().parent() is None:  # Top-levels
                 treeiter += 1
-                item = treeiter.value()  # get child folder
-
-                name = volume['path'].replace(USER_HOME + os.path.sep, '')
-                if volume['type'] == self.ui.backend.SHARE_TYPE:
-                    name = volume['name']
-                self.assertEqual(unicode(item.text(gui.FOLDER_NAME_COL)), name)
-
-                actual_name = unicode(self.ui.widget_items[widget].text(0))
-                if volume['type'] != self.ui.backend.ROOT_TYPE:
-                    self.assertIsInstance(widget, QtGui.QCheckBox)
-                    self.assertEqual(actual_name, name)
-                    widget = widget.nextInFocusChain()
-
-                if not self.ui.remote_folders:
-                    self.assertIsInstance(widget, QtGui.QPushButton)
-                    self.assertEqual(actual_name, name)
-                    widget = widget.nextInFocusChain()
-
+                continue
+            if self.ui.remote_folders:
+                self.assertIsInstance(widget, QtGui.QCheckBox)
+            # ROOT volume has no checkbox
+            elif treeiter.value().text(1) or self.ui.remote_folders:
+                self.assertIsInstance(widget, QtGui.QPushButton)
+            else:
+                self.assertIsInstance(widget, QtGui.QCheckBox)
+                widget = widget.nextInFocusChain()
+                self.assertIsInstance(widget, QtGui.QPushButton)
+            widget = widget.nextInFocusChain()
             treeiter += 1
-            item = treeiter.value()
 
     def test_widget_dict(self):
         """Ensure the widget_items dictionary is full."""
@@ -995,13 +980,17 @@
     def assert_folder_added(self, path, volume_id=None, item=None):
         """A entry for 'path' was added to the folder list."""
         folders = self.ui.ui.folders
-        if item is None:  # grab the last one added
-            item = folders.topLevelItem(folders.topLevelItemCount() - 1)
 
         if path == MUSIC_PATH:
             display_name = gui.MUSIC_DISPLAY_NAME
         else:
             display_name = path.replace(USER_HOME + os.path.sep, '')
+        if item is None:  # search for it
+            items = folders.findItems(display_name,
+                QtCore.Qt.MatchFixedString | QtCore.Qt.MatchCaseSensitive,
+                gui.FOLDER_NAME_COL)
+            self.assertEqual(len(items), 1)
+            item = items[0]
         self.assertEqual(display_name,
                          unicode(item.text(gui.LOCAL_SUBSCRIPTION_COL)))
 
@@ -1094,10 +1083,6 @@
         # the following will execute update_sizes once
         # will fill in all the path sizes for the already added folders
         self.timer.timeout.emit()
-
-        folders = self.ui.ui.folders
-        treeiter = gui.QtGui.QTreeWidgetItemIterator(folders)
-
         volumes = []
         for _, _, vols in volumes_info:
             volumes.extend(vols)
@@ -1108,28 +1093,14 @@
                 not bool(volume['subscribed'])):
                 # non-folders or unsubscribed should not appear in the tree
                 continue
-
-            item = treeiter.value()
-            self.assertTrue(item is not None)
             self.assert_folder_added(volume['path'],
-                                     volume_id=volume['volume_id'], item=item)
-
-            treeiter += 1
-            item = treeiter.value()
-
+                                     volume_id=volume['volume_id'])
         for folder in gui.default_folders(USER_HOME):
             valid = yield self.validate_path(folder)
             if not valid:
                 continue  # invalid folders should not appear in the tree
-
-            item = treeiter.value()
-            self.assertTrue(item is not None)
-            self.assert_folder_added(folder, volume_id=None, item=item)
-
-            treeiter += 1
-            item = treeiter.value()
-
-        self.assertTrue(item is None)
+            self.assert_folder_added(folder, volume_id=None, item=None)
+
         self.assert_space_header_correct(ACCOUNT_INFO['quota_used'])
 
     @defer.inlineCallbacks