Merge lp:~ralsina/ubuntuone-control-panel/check_subscribe_errors into lp:ubuntuone-control-panel

[Roberto Alsina]

 - Give an error if the user subscribes a UDF and the local path is not a folder (Fixes LP:1033488)

Text approved by Robert Grant.

To test IRL, create a UDF, then unsubscribe from it, remove it from disk, and create a file in its place.
When you try to subscribe to it again, it will give an error.

For another test: create a UDF, unsubscribe it, remove it from disk, and create a valid symlink pointing to a folder. When you try to subscribe to it again, it will give an error.

[lp:~ralsina/ubuntuone-control-panel/check_subscribe_errors]

341. By Roberto Alsina on 2012-08-06

untypo

[Eric Casteleijn (thisfred)]

+1 with s/already exists in your device/already exists on your device/

review: Approve

[Brian Curtin (brian.curtin)]

review: Approve

[Alejandro J. Cura (alecu)]

When the symlink points to a folder, it gets wrongly detected as a folder, and then an IPCError occurs.

review: Needs Fixing

[Alejandro J. Cura (alecu)]

Just to be clear, a symlink pointing to a folder is not possible right now within syncdaemon, because that folder could be in a different filesystem, or on a removable drive, etc, etc.

[lp:~ralsina/ubuntuone-control-panel/check_subscribe_errors]

342. By Roberto Alsina on 2012-08-07

add test for when you are subscribing to a folder that exists as a valid symlink pointing to a folder

343. By Roberto Alsina on 2012-08-07

handle case when you are subscribing to a folder that exists as a valid symlink pointing to a folder

[Roberto Alsina (ralsina)]

Added test and handler for the case where the folder the user tries to subscribe is a symlink that points to a real folder.

I think this covers all possible combinations ;-)

[Alejandro J. Cura (alecu)]

Works exactly as described!

review: Approve

=== modified file 'ubuntuone/controlpanel/gui/__init__.py'
--- ubuntuone/controlpanel/gui/__init__.py	2012-05-29 14:36:17 +0000
+++ ubuntuone/controlpanel/gui/__init__.py	2012-08-07 01:39:17 +0000
@@ -140,6 +140,9 @@
                           'with your local folder "%(folder_path)s" when '
                           'subscribing.\nDo you want to subscribe to this '
                           'cloud folder?')
+FOLDER_EXISTS_AS_FILE = _('Unable to subscribe because '
+                         '"%(folder_path)s" already exists on your '
+                         'device and is not a folder.')
 FOLDERS_MANAGE_LABEL = _('Go to the web for public and private sharing '
     'options')
 FOLDERS_TITLE = _('Select which folders from your cloud you want to sync with '

=== modified file 'ubuntuone/controlpanel/gui/qt/folders.py'
--- ubuntuone/controlpanel/gui/qt/folders.py	2012-06-04 16:25:39 +0000
+++ ubuntuone/controlpanel/gui/qt/folders.py	2012-08-07 01:39:17 +0000
@@ -26,6 +26,8 @@
 from PyQt4 import QtGui, QtCore
 from twisted.internet import defer
 
+from ubuntuone.platform import is_link
+
 from ubuntuone.controlpanel.utils import default_folders
 from ubuntuone.controlpanel.logger import setup_logging, log_call
 from ubuntuone.controlpanel.gui import (
@@ -38,6 +40,7 @@
     FOLDERS_COLUMN_NAME,
     FOLDERS_COLUMN_SYNC_LOCALLY,
     FOLDERS_CONFIRM_MERGE,
+    FOLDER_EXISTS_AS_FILE,
     FOLDERS_MANAGE_LABEL,
     GET_MORE_STORAGE,
     humanize,
@@ -328,16 +331,23 @@
                     os.path.exists(volume_path))
         response = YES
         if subscribed and os.path.exists(volume_path):
-            text = FOLDERS_CONFIRM_MERGE % {'folder_path': volume_path}
-            buttons = YES | NO | CANCEL
-            response = QtGui.QMessageBox.warning(self, '', text, buttons, YES)
+            if os.path.isdir(volume_path) and not is_link(volume_path):
+                text = FOLDERS_CONFIRM_MERGE % {'folder_path': volume_path}
+                buttons = YES | NO | CANCEL
+                response = QtGui.QMessageBox.warning(self, '', text, buttons,
+                    YES)
+            else:
+                text = FOLDER_EXISTS_AS_FILE % {'folder_path': volume_path}
+                buttons = CLOSE
+                QtGui.QMessageBox.warning(self, '', text, buttons)
+                response = NO
 
         self.is_processing = True
 
         if response == YES:
             # user accepted, merge the folder content
             yield self.backend.change_volume_settings(volume_id,
-                                                {'subscribed': subscribed})
+                                            {'subscribed': subscribed})
             self.load()
         else:
             # restore old value

=== modified file 'ubuntuone/controlpanel/gui/qt/tests/test_folders.py'
--- ubuntuone/controlpanel/gui/qt/tests/test_folders.py	2012-06-22 14:42:29 +0000
+++ ubuntuone/controlpanel/gui/qt/tests/test_folders.py	2012-08-07 01:39:17 +0000
@@ -591,6 +591,8 @@
     def test_confirm_dialog_if_path_exists(self):
         """The confirmation dialog is correct."""
         self.patch(gui.os.path, 'exists', lambda path: True)
+        self.patch(gui.os.path, 'isdir', lambda path: True)
+        self.patch(gui, 'is_link', lambda path: False)
 
         # make sure the item is subscribed
         self.ui.is_processing = True
@@ -607,6 +609,48 @@
         self.assertEqual(FakedDialog.kwargs, {})
 
     @defer.inlineCallbacks
+    def test_error_if_path_exists_as_file(self):
+        """The error dialog is correct."""
+        self.patch(gui.os.path, 'exists', lambda path: True)
+        self.patch(gui.os.path, 'isdir', lambda path: False)
+        self.patch(gui, 'is_link', lambda path: False)
+
+        # make sure the item is subscribed
+        self.ui.is_processing = True
+        self.set_item_checked()
+        self.ui.is_processing = False
+
+        yield self.ui.on_folders_itemChanged(self.item)
+
+        volume_path = self.item.volume_path
+        msg = gui.FOLDER_EXISTS_AS_FILE % {'folder_path': volume_path}
+        buttons = gui.CLOSE
+        self.assertEqual(FakedDialog.args,
+                         (self.ui, '', msg, buttons))
+        self.assertEqual(FakedDialog.kwargs, {})
+
+    @defer.inlineCallbacks
+    def test_error_if_path_exists_as_valid_link(self):
+        """The error dialog is correct."""
+        self.patch(gui.os.path, 'exists', lambda path: True)
+        self.patch(gui.os.path, 'isdir', lambda path: True)
+        self.patch(gui, 'is_link', lambda path: True)
+
+        # make sure the item is subscribed
+        self.ui.is_processing = True
+        self.set_item_checked()
+        self.ui.is_processing = False
+
+        yield self.ui.on_folders_itemChanged(self.item)
+
+        volume_path = self.item.volume_path
+        msg = gui.FOLDER_EXISTS_AS_FILE % {'folder_path': volume_path}
+        buttons = gui.CLOSE
+        self.assertEqual(FakedDialog.args,
+                         (self.ui, '', msg, buttons))
+        self.assertEqual(FakedDialog.kwargs, {})
+
+    @defer.inlineCallbacks
     def test_confirm_dialog_if_path_does_not_exist(self):
         """The confirmation dialog is not created if not needed."""
         self.patch(gui.os.path, 'exists', lambda path: False)