Merge lp:~nataliabidart/ubuntuone-control-panel/better-path-handling into lp:ubuntuone-control-panel

[Natalia Bidart]

- Do a more robust path handling when double clicking on the volumes tree view (LP: #728601, LP: #718407).

[Roberto Alsina (ralsina)]

+1 yes, this is more robust, but something is broken on syncdaemon if it's passing None there.

review: Approve

[Eric Casteleijn (thisfred)]

Looks good

review: Approve

[Natalia Bidart (nataliabidart)]

The attempt to merge lp:~nataliabidart/ubuntuone-control-panel/better-path-handling into lp:ubuntuone-control-panel failed. Below is the output from the failed tests.

Running test suite for ubuntuone/controlpanel
Xlib: extension "RANDR" missing on display ":99".
ubuntuone.controlpanel.gtk.tests
  BaseTestCase
    runTest ... [OK]
ubuntuone.controlpanel.gtk.tests.test_gui_basic
  ControlPanelMixinTestCase
    test_is_a_control_panel_mixin ... [OK]
    test_ui_can_be_created ... [OK]
  ControlPanelTestCase
    test_backend_is_shutdown_on_close ... [OK]
    test_credentials_found_connects_syncdaemon ... [OK]
    test_credentials_found_shows_dashboard_panel ... [OK]
    test_credentials_found_shows_services_panel ... [OK]
    test_is_a_notebook ... [OK]
    test_local_device_removed_shows_overview_panel ... [OK]
    test_main_window_is_passed_to_child ... [OK]
    test_on_show_management_panel ... [OK]
    test_on_show_management_panel_is_idempotent ... [OK]
    test_overview_is_shown_at_startup ... [OK]
    test_startup_props ... [OK]
    test_startup_visibility ... [OK]
  ControlPanelWindowAlertParamTestCase
    test_alert ... [OK]
  ControlPanelWindowInvalidParamsTestCase
    test_closing_stops_the_main_lopp ... [OK]
    test_control_panel_is_the_only_child ... [OK]
    test_icon_name_is_correct ... [OK]
    test_is_a_window ... [OK]
    test_main_start_gtk_main_loop ... [OK]
    test_main_window_is_passed_to_child ... [OK]
    test_max_size ... [OK]
    test_startup_visibility ... [OK]
    test_switch_to ... [OK]
    test_title_is_correct ... [OK]
  ControlPanelWindowParamsNoneTestCase
    test_closing_stops_the_main_lopp ... [OK]
    test_control_panel_is_the_only_child ... [OK]
    test_icon_name_is_correct ... [OK]
    test_is_a_window ... [OK]
    test_main_start_gtk_main_loop ... [OK]
    test_main_window_is_passed_to_child ... [OK]
    test_max_size ... [OK]
    test_startup_visibility ...

=== modified file 'ubuntuone/controlpanel/gtk/gui.py'
--- ubuntuone/controlpanel/gtk/gui.py	2011-04-04 20:36:59 +0000
+++ ubuntuone/controlpanel/gtk/gui.py	2011-04-04 22:35:48 +0000
@@ -551,6 +551,10 @@
                 elif name == self.MUSIC_DISPLAY_NAME:
                     icon_name = self.MUSIC_ICON_NAME
 
+                if volume[u'path'] is None:
+                    logger.warning('on_volumes_info_ready: about to store a '
+                                   'volume with None path: %r', volume)
+
                 row = (name, bool(volume[u'subscribed']), icon_name, True,
                        sensitive, gtk.ICON_SIZE_MENU, volume['volume_id'],
                        volume[u'path'])
@@ -607,7 +611,14 @@
         """The user double clicked on a row."""
         treeiter = self.volumes_store.get_iter(path)
         volume_path = self.volumes_store.get_value(treeiter, 7)
-        uri_hook(None, FILE_URI_PREFIX + volume_path)
+        if volume_path is None:
+            logger.warning('on_volumes_view_row_activated: volume_path for '
+                           'tree_path %r is None', path)
+        elif not os.path.exists(volume_path):
+            logger.warning('on_volumes_view_row_activated: path %r '
+                           'does not exist', volume_path)
+        else:
+            uri_hook(None, FILE_URI_PREFIX + volume_path)
 
     def load(self):
         """Load the volume list."""

=== modified file 'ubuntuone/controlpanel/gtk/tests/test_gui.py'
--- ubuntuone/controlpanel/gtk/tests/test_gui.py	2011-04-04 18:11:34 +0000
+++ ubuntuone/controlpanel/gtk/tests/test_gui.py	2011-04-04 22:35:48 +0000
@@ -278,6 +278,7 @@
 
     def test_clicking_on_row_opens_folder(self):
         """The folder activated is opened."""
+        self.patch(gui.os.path, 'exists', lambda *a: True)
         self.patch(gui, 'uri_hook', self._set_called)
         self.ui.on_volumes_info_ready(FAKE_VOLUMES_INFO)
 
@@ -287,6 +288,33 @@
         self.assertEqual(self._called,
                          ((None, gui.FILE_URI_PREFIX + ROOT['path']), {}))
 
+    def test_clicking_on_row_handles_path_none(self):
+        """None paths are properly handled."""
+        self.patch(gui, 'uri_hook', self._set_called)
+        self.patch(self.ui.volumes_store, 'get_value', lambda *a: None)
+        self.ui.on_volumes_info_ready(FAKE_VOLUMES_INFO)
+
+        self.ui.volumes_view.row_activated('0:0',
+                                           self.ui.volumes_view.get_column(0))
+
+        self.assertTrue(self.memento.check_warning('tree_path (0, 0)',
+                                                   'volume_path', 'is None'))
+        self.assertEqual(self._called, False)
+
+    def test_clicking_on_row_handles_path_non_existent(self):
+        """Not-existent paths are properly handled."""
+        self.patch(gui.os.path, 'exists', lambda *a: False)
+        self.patch(gui, 'uri_hook', self._set_called)
+        path = 'not-in-disk'
+        self.patch(self.ui.volumes_store, 'get_value', lambda *a: path)
+        self.ui.on_volumes_info_ready(FAKE_VOLUMES_INFO)
+
+        self.ui.volumes_view.row_activated('0:0',
+                                           self.ui.volumes_view.get_column(0))
+
+        self.assertTrue(self.memento.check_warning(path, 'does not exist'))
+        self.assertEqual(self._called, False)
+
     def test_on_volumes_info_ready_with_music_folder(self):
         """The volumes info is processed when ready."""
         info = [(u'', u'147852369', [ROOT] + [MUSIC_FOLDER])]