Merge lp:~nataliabidart/ubuntuone-control-panel/warm-folder-merge into lp:ubuntuone-control-panel

[Natalia Bidart]

- Added warning message when user is about to subscribe to an UDF that already exists on disk (LP: #674462).

To review, you should try IRL:

* subscribe to a folder which path does exist on disk (merge confirmation requested)
* subscribe to a folder which path does not exist on disk (no confirmation requested)
* unsubscribe from a folder (no confirmation requested)

[lp:~nataliabidart/ubuntuone-control-panel/warm-folder-merge]

84. By Natalia Bidart on 2011-02-28

Fixed typo.

[Roberto Alsina (ralsina)]

+1 (code review, no fieldtest)

review: Approve

[Eric Casteleijn (thisfred)]

Looks good, popup shows when needed. I do see an error (Value could not be retrieved) after I unselect a cloud folder, and the Cloud Folder tab greys out completely. Switching to another tab and back fixes it. Probably not related to this branch.

review: Approve

=== modified file 'ubuntuone/controlpanel/gtk/gui.py'
--- ubuntuone/controlpanel/gtk/gui.py	2011-02-24 12:24:20 +0000
+++ ubuntuone/controlpanel/gtk/gui.py	2011-02-28 20:05:32 +0000
@@ -394,6 +394,9 @@
     FREE_SPACE = _('%(free_space)s available storage')
     NO_VOLUMES = _('No folders to show.')
     NAME_NOT_SET = _('[unknown user name]')
+    CONFIRM_MERGE = _('The contents of your cloud folder will be merged with '
+                      'your local folder "%(folder_path)s" when subscribing.\n'
+                      'Do you want to subscribe to this cloud folder?')
 
     MAX_COLS = 8
 
@@ -410,6 +413,12 @@
         self.add(self.itself)
         self.show_all()
 
+        kw = dict(parent=main_window,
+                  flags=gtk.DIALOG_MODAL | gtk.DIALOG_DESTROY_WITH_PARENT,
+                  type=gtk.MESSAGE_WARNING,
+                  buttons=gtk.BUTTONS_YES_NO)
+        self.confirm_dialog = gtk.MessageDialog(**kw)
+
         # name, subscribed, icon name, show toggle, sensitive, icon size,
         # id, path
         self._empty_row = ('', False, '', False, False, gtk.ICON_SIZE_MENU,
@@ -506,15 +515,24 @@
         """The user toggled 'widget'."""
         treeiter = self.volumes_store.get_iter(path)
         volume_id = self.volumes_store.get_value(treeiter, 6)
-        subscribed = not self.volumes_store.get_value(treeiter, 1)
-
-        self.volumes_store.set_value(treeiter, 1, subscribed)
-
-        self.backend.change_volume_settings(volume_id,
-            {'subscribed': bool_str(subscribed)},
-            reply_handler=NO_OP, error_handler=error_handler)
-
-        self.is_processing = True
+        volume_path = self.volumes_store.get_value(treeiter, 7)
+        subscribed = self.volumes_store.get_value(treeiter, 1)
+
+        response = gtk.RESPONSE_YES
+        if not subscribed and os.path.exists(volume_path):
+            self.confirm_dialog.set_markup(self.CONFIRM_MERGE %
+                                           {'folder_path': volume_path})
+            response = self.confirm_dialog.run()
+            self.confirm_dialog.hide()
+
+        if response == gtk.RESPONSE_YES:
+            subscribed = not subscribed
+            self.volumes_store.set_value(treeiter, 1, subscribed)
+            self.backend.change_volume_settings(volume_id,
+                {'subscribed': bool_str(subscribed)},
+                reply_handler=NO_OP, error_handler=error_handler)
+
+            self.is_processing = True
 
     def on_volumes_view_row_activated(self, widget, path, *args, **kwargs):
         """The user double clicked on a row."""

=== modified file 'ubuntuone/controlpanel/gtk/tests/__init__.py'
--- ubuntuone/controlpanel/gtk/tests/__init__.py	2011-02-16 19:02:02 +0000
+++ ubuntuone/controlpanel/gtk/tests/__init__.py	2011-02-28 20:05:32 +0000
@@ -18,11 +18,20 @@
 
 """The test suite for the GTK UI for the control panel for Ubuntu One."""
 
+import logging
+
 from collections import defaultdict
 
+from ubuntuone.devtools.handlers import MementoHandler
+
 from ubuntuone.controlpanel.gtk import gui
 from ubuntuone.controlpanel.gtk.tests.test_package_manager import (
     FakedTransaction)
+from ubuntuone.controlpanel.tests import TestCase
+
+
+# Attribute 'yyy' defined outside __init__, access to a protected member
+# pylint: disable=W0201, W0212
 
 
 FAKE_ACCOUNT_INFO = {'type': 'Payed', 'name': 'Test me',
@@ -207,7 +216,8 @@
         self._args = args
         self._kwargs = kwargs
         self.was_run = False
-        self.is_visible = True
+        self.is_visible = False
+        self.markup = kwargs.get('message_format', None)
         self.show = lambda: setattr(self, 'is_visible', True)
         self.hide = lambda: setattr(self, 'is_visible', False)
         self.response_code = None
@@ -216,3 +226,67 @@
         """Set flag and return 'self.response_code'."""
         self.was_run = True
         return self.response_code
+
+    def set_markup(self, msg):
+        """Set the markup."""
+        self.markup = msg
+
+
+class BaseTestCase(TestCase):
+    """Basics for testing."""
+
+    # self.klass is not callable
+    # pylint: disable=E1102
+    klass = None
+    kwargs = {}
+
+    def setUp(self):
+        super(BaseTestCase, self).setUp()
+        self.patch(gui.os.path, 'expanduser',
+                   lambda path: path.replace('~', USER_HOME))
+        self.patch(gui.gtk, 'main', lambda: None)
+        self.patch(gui.gtk, 'MessageDialog', FakedConfirmDialog)
+        self.patch(gui.dbus, 'SessionBus', FakedSessionBus)
+        self.patch(gui.dbus, 'Interface', FakedInterface)
+        self.patch(gui.networkstate, 'NetworkManagerState', FakedNMState)
+        self.patch(gui.package_manager, 'PackageManager', FakedPackageManager)
+
+        if self.klass is not None:
+            self.ui = self.klass(**self.kwargs)
+
+        self.memento = MementoHandler()
+        self.memento.setLevel(logging.DEBUG)
+        gui.logger.addHandler(self.memento)
+
+    def tearDown(self):
+        try:
+            self.ui.hide()
+            del self.ui
+            self.ui = None
+        except AttributeError:
+            pass
+        super(BaseTestCase, self).tearDown()
+
+    def assert_image_equal(self, image, filename):
+        """Check that expected and actual represent the same image."""
+        pb = gui.gtk.gdk.pixbuf_new_from_file(gui.get_data_file(filename))
+        self.assertEqual(image.get_pixbuf().get_pixels(), pb.get_pixels())
+
+    def assert_backend_called(self, method_name, args, backend=None):
+        """Check that the control panel backend 'method_name' was called."""
+        if backend is None:
+            backend = self.ui.backend
+        self.assertIn(method_name, backend._called)
+        kwargs = {'reply_handler': gui.NO_OP,
+                  'error_handler': gui.error_handler}
+        self.assertEqual(backend._called[method_name], (args, kwargs))
+
+    def assert_warning_correct(self, warning, text):
+        """Check that 'warning' is visible, showing 'text'."""
+        self.assertTrue(warning.get_visible(), 'Must be visible.')
+        self.assertEqual(warning.get_label(), gui.WARNING_MARKUP % text)
+
+    def assert_function_decorated(self, decorator, func):
+        """Check that 'func' is decorated with 'decorator'."""
+        expected = decorator(lambda: None)
+        self.assertEqual(expected.func_code, func.im_func.func_code)

=== modified file 'ubuntuone/controlpanel/gtk/tests/test_gui.py'
--- ubuntuone/controlpanel/gtk/tests/test_gui.py	2011-02-23 21:23:30 +0000
+++ ubuntuone/controlpanel/gtk/tests/test_gui.py	2011-02-28 20:05:32 +0000
@@ -20,20 +20,15 @@
 
 from __future__ import division
 
-import logging
-
-from ubuntuone.devtools.handlers import MementoHandler
-
 from ubuntuone.controlpanel.gtk import gui
 from ubuntuone.controlpanel.gtk.tests import (FAKE_ACCOUNT_INFO,
-    FAKE_DEVICE_INFO, FAKE_DEVICES_INFO,
+    FAKE_DEVICE_INFO, FAKE_DEVICES_INFO, FAKE_FOLDERS_INFO,
     FAKE_VOLUMES_INFO, FAKE_REPLICATIONS_INFO, ROOT, USER_HOME,
-    FakedNMState, FakedSSOBackend, FakedSessionBus, FakedInterface,
-    FakedPackageManager, FakedConfirmDialog,
+    BaseTestCase, FakedSSOBackend, FakedConfirmDialog,
 )
-from ubuntuone.controlpanel.tests import TOKEN, TestCase
 from ubuntuone.controlpanel.gtk.tests.test_package_manager import (
     SUCCESS, FAILURE)
+from ubuntuone.controlpanel.tests import TOKEN
 
 
 # Attribute 'yyy' defined outside __init__, access to a protected member
@@ -42,66 +37,6 @@
 # pylint: disable=C0302
 
 
-class BaseTestCase(TestCase):
-    """Basics for testing."""
-
-    # self.klass is not callable
-    # pylint: disable=E1102
-    klass = None
-    kwargs = {}
-
-    def setUp(self):
-        super(BaseTestCase, self).setUp()
-        self.patch(gui.os.path, 'expanduser',
-                   lambda path: path.replace('~', USER_HOME))
-        self.patch(gui.gtk, 'main', lambda: None)
-        self.patch(gui.gtk, 'MessageDialog', FakedConfirmDialog)
-        self.patch(gui.dbus, 'SessionBus', FakedSessionBus)
-        self.patch(gui.dbus, 'Interface', FakedInterface)
-        self.patch(gui.networkstate, 'NetworkManagerState', FakedNMState)
-        self.patch(gui.package_manager, 'PackageManager', FakedPackageManager)
-
-        if self.klass is not None:
-            self.ui = self.klass(**self.kwargs)
-
-        self.memento = MementoHandler()
-        self.memento.setLevel(logging.DEBUG)
-        gui.logger.addHandler(self.memento)
-
-    def tearDown(self):
-        try:
-            self.ui.hide()
-            del self.ui
-            self.ui = None
-        except AttributeError:
-            pass
-        super(BaseTestCase, self).tearDown()
-
-    def assert_image_equal(self, image, filename):
-        """Check that expected and actual represent the same image."""
-        pb = gui.gtk.gdk.pixbuf_new_from_file(gui.get_data_file(filename))
-        self.assertEqual(image.get_pixbuf().get_pixels(), pb.get_pixels())
-
-    def assert_backend_called(self, method_name, args, backend=None):
-        """Check that the control panel backend 'method_name' was called."""
-        if backend is None:
-            backend = self.ui.backend
-        self.assertIn(method_name, backend._called)
-        kwargs = {'reply_handler': gui.NO_OP,
-                  'error_handler': gui.error_handler}
-        self.assertEqual(backend._called[method_name], (args, kwargs))
-
-    def assert_warning_correct(self, warning, text):
-        """Check that 'warning' is visible, showing 'text'."""
-        self.assertTrue(warning.get_visible(), 'Must be visible.')
-        self.assertEqual(warning.get_label(), gui.WARNING_MARKUP % text)
-
-    def assert_function_decorated(self, decorator, func):
-        """Check that 'func' is decorated with 'decorator'."""
-        expected = decorator(lambda: None)
-        self.assertEqual(expected.func_code, func.im_func.func_code)
-
-
 class ControlPanelMixinTestCase(BaseTestCase):
     """The test suite for the control panel widget."""
 
@@ -896,10 +831,32 @@
         self.test_on_volumes_info_error()
         self.test_on_volumes_info_ready_with_no_volumes()
 
+    def test_clicking_on_row_opens_folder(self):
+        """The folder activated is opened."""
+        self.patch(gui, 'uri_hook', self._set_called)
+        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.assertEqual(self._called,
+                         ((None, gui.FILE_URI_PREFIX + ROOT['path']), {}))
+
+
+class VolumesSubscriptionTestCase(VolumesTestCase):
+    """The test suite for the volumes panel."""
+
+    kwargs = {'main_window': object()}
+    tree_path = '0:3'  # this is the /home/tester/foo folder, not subscribed
+
+    def setUp(self):
+        super(VolumesSubscriptionTestCase, self).setUp()
+        self.patch(gui.os.path, 'exists', lambda path: True)
+        self.ui.confirm_dialog.response_code = gui.gtk.RESPONSE_YES
+        self.ui.on_volumes_info_ready(FAKE_VOLUMES_INFO)
+
     def test_on_subscribed_toggled(self):
         """Clicking on 'subscribed' updates the folder subscription."""
-        self.ui.on_volumes_info_ready(FAKE_VOLUMES_INFO)
-
         real_rows = len(FAKE_VOLUMES_INFO)
         data = zip(range(real_rows)[::2], FAKE_VOLUMES_INFO)  # skip emtpy rows
         for parent, (_, _, volumes) in data:
@@ -930,8 +887,7 @@
 
     def test_on_volume_setting_changed(self):
         """The setting for a volume was successfully changed."""
-        self.ui.on_volumes_info_ready(FAKE_VOLUMES_INFO)
-        self.ui.on_subscribed_toggled(None, "0:0")
+        self.ui.on_subscribed_toggled(None, self.tree_path)
 
         self.ui.on_volume_settings_changed(volume_id=None)  # id not used
 
@@ -940,8 +896,7 @@
 
     def test_on_volume_setting_change_error(self):
         """The setting for a volume was not successfully changed."""
-        self.ui.on_volumes_info_ready(FAKE_VOLUMES_INFO)
-        self.ui.on_subscribed_toggled(None, "0:0")
+        self.ui.on_subscribed_toggled(None, self.tree_path)
 
         self.patch(self.ui, 'load', self._set_called)
         self.ui.on_volume_settings_change_error(volume_id=None,
@@ -949,16 +904,66 @@
         # reload folders list to sanitize the info in volumes_store
         self.assertTrue(self._called, ((), {}))
 
-    def test_clicking_on_row_opens_folder(self):
-        """The folder activated is opened."""
-        self.patch(gui, 'uri_hook', self._set_called)
-        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.assertEqual(self._called,
-                         ((None, gui.FILE_URI_PREFIX + ROOT['path']), {}))
+    def test_confirm_dialog(self):
+        """The confirmation dialog is correct."""
+        dialog = self.ui.confirm_dialog
+
+        self.assertEqual(dialog._args, ())
+        flags = gui.gtk.DIALOG_MODAL | gui.gtk.DIALOG_DESTROY_WITH_PARENT
+        kwargs = dict(parent=self.kwargs['main_window'],
+                      flags=flags, type=gui.gtk.MESSAGE_WARNING,
+                      buttons=gui.gtk.BUTTONS_YES_NO)
+        self.assertEqual(dialog._kwargs, kwargs)
+
+    def test_subscribe_shows_confirmation_dialog(self):
+        """Clicking on subscribe displays a confirmation dialog."""
+        self.ui.on_subscribed_toggled(None, self.tree_path)
+
+        path = FAKE_FOLDERS_INFO[0]['path']
+        self.assertTrue(self.ui.confirm_dialog.was_run, 'dialog was run')
+        self.assertEqual(self.ui.confirm_dialog.markup,
+                         self.ui.CONFIRM_MERGE % {'folder_path': path})
+        self.assertFalse(self.ui.confirm_dialog.is_visible, 'dialog was hid')
+
+    def test_subscribe_does_not_call_backend_if_dialog_closed(self):
+        """Backend is not called if users closes the confirmation dialog."""
+        self.ui.confirm_dialog.response_code = gui.gtk.RESPONSE_DELETE_EVENT
+        self.ui.on_subscribed_toggled(None, self.tree_path)
+
+        self.assertNotIn('change_volume_settings', self.ui.backend._called)
+        self.assertFalse(self.ui.is_processing)
+
+    def test_subscribe_does_not_call_backend_if_answer_is_no(self):
+        """Backend is not called if users clicks on 'No'."""
+        self.ui.confirm_dialog.response_code = gui.gtk.RESPONSE_NO
+        self.ui.on_subscribed_toggled(None, self.tree_path)
+
+        self.assertNotIn('change_volume_settings', self.ui.backend._called)
+        self.assertFalse(self.ui.is_processing)
+
+    def test_no_confirmation_if_no_local_folder(self):
+        """The confirmation dialog is not shown if local folder not present."""
+        self.patch(gui.os.path, 'exists', lambda path: False)
+        self.ui.on_subscribed_toggled(None, self.tree_path)
+
+        self.assertFalse(self.ui.confirm_dialog.was_run, 'dialog was not run')
+        self.assertFalse(self.ui.confirm_dialog.is_visible, 'dialog was hid')
+
+    def test_no_confirmation_if_unsubscribing(self):
+        """The confirmation dialog is not shown if unsubscribing."""
+        self.ui.on_subscribed_toggled(None, self.tree_path)
+
+        treeiter = self.ui.volumes_store.get_iter(self.tree_path)
+        assert self.ui.volumes_store.get_value(treeiter, 1)
+
+        # reset flags
+        self.ui.confirm_dialog.was_run = False
+        self.ui.confirm_dialog.is_visible = False
+
+        self.ui.on_subscribed_toggled(None, self.tree_path)
+
+        self.assertFalse(self.ui.confirm_dialog.was_run, 'dialog was not run')
+        self.assertFalse(self.ui.confirm_dialog.is_visible, 'dialog was hid')
 
 
 class DeviceTestCase(ControlPanelMixinTestCase):