Merge lp:~nataliabidart/magicicada-gui/remove-folders into lp:magicicada-gui

[Natalia Bidart]

- Fix how folder op results are handled (LP: #899305).
- Allow folder removal (LP: #568173).

[Facundo Batista (facundo)]

+1

review: Approve

[Natalia Bidart (nataliabidart)]

There are additional revisions which have not been approved in review. Please seek review and approval of these new revisions.

=== modified file 'data/ui/folders.ui'
--- data/ui/folders.ui	2011-10-11 23:51:51 +0000
+++ data/ui/folders.ui	2011-12-06 00:43:23 +0000
@@ -11,6 +11,15 @@
     <property name="use_stock">True</property>
     <signal name="clicked" handler="on_add_folder_button_clicked" swapped="no"/>
   </object>
+  <object class="GtkButton" id="remove_folder_button">
+    <property name="label">gtk-remove</property>
+    <property name="visible">True</property>
+    <property name="can_focus">True</property>
+    <property name="receives_default">True</property>
+    <property name="use_action_appearance">False</property>
+    <property name="use_stock">True</property>
+    <signal name="clicked" handler="on_remove_folder_button_clicked" swapped="no"/>
+  </object>
   <object class="GtkScrolledWindow" id="root">
     <property name="visible">True</property>
     <property name="can_focus">True</property>
@@ -25,6 +34,7 @@
         <property name="search_column">0</property>
         <property name="enable_grid_lines">both</property>
         <property name="enable_tree_lines">True</property>
+        <signal name="cursor-changed" handler="on_view_cursor_changed" swapped="no"/>
         <child>
           <object class="GtkTreeViewColumn" id="folders_path">
             <property name="resizable">True</property>

=== modified file 'magicicada/dbusiface.py'
--- magicicada/dbusiface.py	2011-11-19 17:14:50 +0000
+++ magicicada/dbusiface.py	2011-12-06 00:43:23 +0000
@@ -558,7 +558,7 @@
             raise FolderOperationError(**kwargs)
         else:
             logger.debug("%s folder finished ok: %s", act_name, result)
-            folder = self._get_folder_data(result[0])
+            folder = self._get_folder_data(result)
             defer.returnValue(folder)
 
     def create_folder(self, path):

=== modified file 'magicicada/gui/gtk/listings.py'
--- magicicada/gui/gtk/listings.py	2011-10-11 23:51:51 +0000
+++ magicicada/gui/gtk/listings.py	2011-12-06 00:43:23 +0000
@@ -33,6 +33,12 @@
 
 
 ADD_NEW_FOLDER = _('Add a new folder')
+ARE_YOU_SURE = _('Are you sure?')
+ARE_YOU_SURE_REMOVE_FOLDER = _('Are you sure you want to stop syncing '
+                               'the folder %s?')
+ARE_YOU_SURE_REMOVE_FOLDER_SECONDARY_TEXT = _('You will not loose any local '
+    ' data, but the contents of this folder will no longer be synced to/from '
+    ' your cloud in any of your registered devices.')
 ERROR_MESSAGE_MARKUP = '<span foreground="red" font_weight="bold">%s</span>'
 ERROR_MESSAGE = _('Oops! Something went wrong%(details)s')
 
@@ -60,6 +66,13 @@
         gtk.Dialog.__init__(self, **_kwargs)
         self.close_button = self.get_action_area().get_children()[0]
 
+        kwargs = dict(parent=self,
+                      flags=gtk.DIALOG_MODAL | gtk.DIALOG_DESTROY_WITH_PARENT,
+                      type=gtk.MESSAGE_QUESTION,
+                      buttons=gtk.BUTTONS_YES_NO)
+        self.confirm_dialog = gtk.MessageDialog(**kwargs)
+        self.confirm_dialog.set_title(ARE_YOU_SURE)
+
         self.warning_label = gtk.Label()
         self.warning_label.set_selectable(True)
         self.warning_label.set_line_wrap(True)
@@ -142,6 +155,31 @@
         msg = ERROR_MESSAGE % dict(details=' (%s)' % error)
         self.warning_label.set_markup(ERROR_MESSAGE_MARKUP % msg)
 
+    @defer.inlineCallbacks
+    def call_async(self, operation, *args, **kwargs):
+        """Call 'operation(*args, **kwargs)' while disabling the ui.
+
+        The result of calling 'operation' is returned.
+
+        All errors are catched and logged, and shown to the user as a red
+        warning.
+
+        """
+        self.warning_label.set_text('')
+        result = None
+        self.set_sensitive(False)
+        logger.debug('call_async: executing %r with args %r and %r',
+                     operation.__name__, args, kwargs)
+        try:
+            result = yield operation(*args, **kwargs)
+        except Exception, e:  # pylint: disable=W0703
+            logger.exception('call_async: %r failed with:',
+                             operation.__name__)
+            self.on_error(e)
+        self.load()
+        self.set_sensitive(True)
+        defer.returnValue(result)
+
 
 class FoldersDialog(ListingDialog):
     """The list of operations over files/folders."""
@@ -154,8 +192,10 @@
 
     def __init__(self, *args, **kwargs):
         super(FoldersDialog, self).__init__(*args, **kwargs)
+        self.remove_folder_button.set_sensitive(False)
         action_area = self.get_action_area()
         action_area.pack_start(self.add_folder_button)
+        action_area.pack_start(self.remove_folder_button)
         action_area.reorder_child(self.close_button, 0)
 
         kwargs = dict(title=ADD_NEW_FOLDER, parent=None,
@@ -179,12 +219,31 @@
         response = self.file_chooser.run()
         self.file_chooser.hide()
         if response == gtk.RESPONSE_ACCEPT:
+            path = self.file_chooser.get_filename()
             # validate path?
-            self.set_sensitive(False)
-            path = self.file_chooser.get_filename()
-            yield self.sd.create_folder(path)
-            self.load()
-            self.set_sensitive(True)
+            yield self.call_async(self.sd.create_folder, path)
+
+    def on_view_cursor_changed(self, view=None):
+        """Enable the remove button if there's a row selected."""
+        selection = self.view.get_selection()
+        can_remove = selection.count_selected_rows() > 0
+        self.remove_folder_button.set_sensitive(can_remove)
+
+    @defer.inlineCallbacks
+    def on_remove_folder_button_clicked(self, button, *args, **kwargs):
+        """The remove_folder button was clicked."""
+        selection = self.view.get_selection()
+        model, tree_iter = selection.get_selected()
+        path = model.get_value(tree_iter, 0)
+        self.confirm_dialog.set_markup(ARE_YOU_SURE_REMOVE_FOLDER % path)
+        msg = ARE_YOU_SURE_REMOVE_FOLDER_SECONDARY_TEXT
+        self.confirm_dialog.format_secondary_text(msg)
+        response = self.confirm_dialog.run()
+        self.confirm_dialog.hide()
+
+        if response == gtk.RESPONSE_YES:
+            volume_id = model.get_value(tree_iter, 2)
+            yield self.call_async(self.sd.delete_folder, volume_id)
 
 
 class SharesToMeDialog(ListingDialog):

=== modified file 'magicicada/gui/gtk/tests/__init__.py'
--- magicicada/gui/gtk/tests/__init__.py	2011-10-11 23:51:51 +0000
+++ magicicada/gui/gtk/tests/__init__.py	2011-12-06 00:43:23 +0000
@@ -206,8 +206,10 @@
     store = None
     ui_class = main.MagicicadaUI
 
+    @defer.inlineCallbacks
     def setUp(self):
         """Init."""
+        yield super(BaseTestCase, self).setUp()
         self.patch(main.syncdaemon, 'SyncDaemon', FakedSyncdaemon)
         self.ui = self.ui_class(**self.kwargs)
 

=== modified file 'magicicada/gui/gtk/tests/test_listings.py'
--- magicicada/gui/gtk/tests/test_listings.py	2011-10-11 23:51:51 +0000
+++ magicicada/gui/gtk/tests/test_listings.py	2011-12-06 00:43:23 +0000
@@ -24,6 +24,9 @@
 
 from magicicada.gui.gtk.listings import (
     ADD_NEW_FOLDER,
+    ARE_YOU_SURE,
+    ARE_YOU_SURE_REMOVE_FOLDER,
+    ARE_YOU_SURE_REMOVE_FOLDER_SECONDARY_TEXT,
     ERROR_MESSAGE,
     ERROR_MESSAGE_MARKUP,
     FoldersButton,
@@ -132,9 +135,10 @@
     items = [TestDataField(), TestDataField(), TestDataField()]
     ui_class = ListingDialog
 
+    @defer.inlineCallbacks
     def setUp(self):
-        """Init."""
         self.patch(gtk, 'FileChooserDialog', FakeDialog)
+        self.patch(gtk, 'MessageDialog', FakeDialog)
 
         if self.ui_class.data_fields is None:
             fakes = (('attr0', lambda s: ''.join(reversed(s))),
@@ -152,7 +156,7 @@
             self.kwargs['builder'] = builder
             self.addCleanup(self.kwargs.pop, 'builder')
 
-        super(ListingDialogTestCase, self).setUp()
+        yield super(ListingDialogTestCase, self).setUp()
 
         setattr(self.ui.sd, self.ui.sd_attr, self.items)
         self.store = self.ui.store
@@ -340,19 +344,83 @@
         self.assertEqual(self.ui.view.get_selection().get_mode(),
                          gtk.SELECTION_SINGLE)
 
-    def test_on_error(self):
-        """On error, show a error dialog."""
-        self.patch(gtk, 'MessageDialog', FakeDialog)
-
-        exc = TypeError('foo')
-        self.ui.on_error(exc)
+    def test_on_error(self, exc=None):
+        """On error 'exc', show a error dialog."""
+        if exc is None:
+            exc = TypeError('foo')
+            self.ui.on_error(exc)
 
         self.assertTrue(self.ui.warning_label.get_visible())
-        error_msg = ERROR_MESSAGE % dict(details=' (foo)')
+        error_msg = ERROR_MESSAGE % dict(details=' (%s)' % exc)
         self.assertEqual(self.ui.warning_label.get_text(), error_msg)
         self.assertEqual(self.ui.warning_label.get_label(),
                          ERROR_MESSAGE_MARKUP % error_msg)
 
+    def test_confirm_dialog(self):
+        """The confirm dialog dialog is properly created."""
+        self.assertIsInstance(self.ui.confirm_dialog, gtk.MessageDialog)
+
+    def test_confirm_dialog_creation_params(self):
+        """The confirm dialog dialog is created with the expected params."""
+        self.assertEqual(self.ui.confirm_dialog.args, ())
+        kwargs = dict(parent=self.ui,
+                      flags=gtk.DIALOG_MODAL | gtk.DIALOG_DESTROY_WITH_PARENT,
+                      type=gtk.MESSAGE_QUESTION,
+                      buttons=gtk.BUTTONS_YES_NO)
+        self.assertEqual(self.ui.confirm_dialog.kwargs, kwargs)
+
+    def test_confirm_dialog_title(self):
+        """The confirm dialog dialog title is correct."""
+        self.assert_method_called(self.ui.confirm_dialog, 'set_title',
+                                  ARE_YOU_SURE)
+
+    @defer.inlineCallbacks
+    def test_call_async_disables_ui(self):
+        """While calling an async operation, the UI is disabled."""
+        self.patch(self.ui, 'load', self._set_called)
+
+        def check():
+            """Check ui sensibility."""
+            result = self.ui.is_sensitive()
+            return defer.succeed(result)
+
+        self.assertTrue(self.ui.is_sensitive(),
+                        'The ui must be enabled before calling the op.')
+
+        was_sensitive = yield self.ui.call_async(check)
+
+        self.assertFalse(was_sensitive,
+                         'The ui must be disabled while calling the op.')
+        self.assertTrue(self.ui.is_sensitive(),
+                        'The ui must be enabled after calling the op.')
+        self.test_warning_label()  # the warning_label is cleared
+        self.assertEqual(self._called, ((), {}), 'load was called')
+
+    @defer.inlineCallbacks
+    def test_call_async_handles_errors(self):
+        """While calling an async operation, errors are catched and logged."""
+        self.patch(self.ui, 'load', self._set_called)
+        msg = 'Crash boom bang'
+        exc = AssertionError(msg)
+
+        def fail_zaraza():
+            """Throw any error."""
+            return defer.fail(exc)
+
+        result = yield self.ui.call_async(fail_zaraza)
+
+        self.assertEqual(result, None)
+        self.assertTrue(self.memento.check_exception(exc.__class__, msg))
+        self.assertTrue(self.memento.check_error(fail_zaraza.__name__))
+        self.test_on_error(exc=exc)
+        self.assertEqual(self._called, ((), {}), 'load was called')
+
+    @defer.inlineCallbacks
+    def test_call_async_success_after_error(self):
+        """Warning messages are cleared after an error."""
+        yield self.ui.call_async(lambda: defer.fail(ValueError()))
+        yield self.test_call_async_disables_ui()
+
 
 class FoldersDialogTestCase(ListingDialogTestCase):
     """UI test cases for folders."""
@@ -405,14 +473,15 @@
         """When the add_folder_button is clicked, the backend is called."""
         self.patch(self.ui, 'load', self._set_called)
         self.patch(self.ui.file_chooser, 'response', gtk.RESPONSE_ACCEPT)
+
+        def check():
+            """Perform a middle check."""
+            self.assertFalse(self.ui.is_sensitive())
+
+        self.patch(self.ui.sd, 'create_folder', check)
+
         path = '~/foo/test/me'
-
-        def check():
-            """Perform a middle check and return path."""
-            self.assertFalse(self.ui.is_sensitive())
-            return path
-
-        self.patch(self.ui.file_chooser, 'get_filename', check)
+        self.patch(self.ui.file_chooser, 'get_filename', lambda: path)
 
         yield self.ui.add_folder_button.clicked()
 
@@ -427,6 +496,81 @@
         self.assertEqual(self.ui.sd.on_folder_op_error_callback,
                          self.ui.on_error)
 
+    def test_remove_folder_button_disabled(self):
+        """At startup, the remove button is disabled."""
+        self.ui.load()
+        self.assertFalse(self.ui.remove_folder_button.get_sensitive())
+
+
+class FoldersDialogRemoveFolderTestCase(FoldersDialogTestCase):
+    """Test case for folder removal."""
+
+    @defer.inlineCallbacks
+    def setUp(self):
+        yield super(FoldersDialogRemoveFolderTestCase, self).setUp()
+        self.ui.load()
+        idx = 0  # the testing folder
+        self.ui.view.set_cursor(idx)
+        _, _, self.path, _, self.volume_id = SAMPLE_FOLDERS[idx]
+
+    def test_remove_folder_button_disabled(self):
+        """When there is no folder selected, the remove button is disabled."""
+        self.ui.view.get_selection().unselect_path('0')
+        self.ui.on_view_cursor_changed()
+        self.assertFalse(self.ui.remove_folder_button.get_sensitive())
+
+    def test_remove_folder_button_enabled(self):
+        """When there is a folder selected, the remove button is enabled."""
+        self.assertTrue(self.ui.remove_folder_button.get_sensitive())
+
+    @defer.inlineCallbacks
+    def test_on_remove_folder_button_clicked_confirm_dialog_is_shown(self):
+        """On remove_folder_button clicked, the confirm_dialog is shown."""
+        yield self.ui.remove_folder_button.clicked()
+
+        msg = ARE_YOU_SURE_REMOVE_FOLDER % self.path
+        self.assert_method_called(self.ui.confirm_dialog, 'set_markup', msg)
+        self.assert_method_called(self.ui.confirm_dialog,
+                                  'format_secondary_text',
+                                  ARE_YOU_SURE_REMOVE_FOLDER_SECONDARY_TEXT)
+        self.assert_method_called(self.ui.confirm_dialog, 'run')
+        self.assert_method_called(self.ui.confirm_dialog, 'hide')
+
+    @defer.inlineCallbacks
+    def test_on_remove_folder_button_clicked_closes_dialog(self):
+        """If the user closes the dialog, nothing is done."""
+        self.patch(self.ui.confirm_dialog, 'response',
+                   gtk.RESPONSE_DELETE_EVENT)
+        yield self.ui.remove_folder_button.clicked()
+
+        self.assert_no_method_called(self.ui.sd)
+        self.assertTrue(self.ui.is_sensitive())
+
+    @defer.inlineCallbacks
+    def test_on_remove_folder_button_clicked_answers_no(self):
+        """If the user answers no, nothing is done."""
+        self.patch(self.ui.confirm_dialog, 'response', gtk.RESPONSE_NO)
+        yield self.ui.remove_folder_button.clicked()
+
+        self.assert_no_method_called(self.ui.sd)
+        self.assertTrue(self.ui.is_sensitive())
+
+    @defer.inlineCallbacks
+    def test_on_remove_folder_button_clicked_answers_yes(self):
+        """If the user answers yes, sd.delete_folder is called."""
+        self.patch(self.ui.confirm_dialog, 'response', gtk.RESPONSE_YES)
+
+        def check():
+            """Perform a middle check."""
+            self.assertFalse(self.ui.is_sensitive())
+
+        self.patch(self.ui.sd, 'delete_folder', check)
+
+        yield self.ui.remove_folder_button.clicked()
+
+        self.assert_method_called(self.ui.sd, 'delete_folder', self.volume_id)
+        self.assertTrue(self.ui.is_sensitive())
+
 
 class SharesToMeDialogTestCase(ListingDialogTestCase):
     """UI test cases for shares_to_me."""
@@ -466,10 +610,10 @@
     stock_id = None
     ui_class = ListingButton
 
+    @defer.inlineCallbacks
     def setUp(self):
-        """Init."""
         self.patch(self.ui_class, 'dialog_class', FakeDialog)
-        super(ListingButtonTestCase, self).setUp()
+        yield super(ListingButtonTestCase, self).setUp()
 
     def test_label(self):
         """The label is correct."""

=== modified file 'magicicada/logger.py'
--- magicicada/logger.py	2011-12-05 21:20:00 +0000
+++ magicicada/logger.py	2011-12-06 00:43:23 +0000
@@ -85,8 +85,8 @@
     handler = CustomRotatingFH(logfile, maxBytes=1e6, backupCount=10)
     logger.addHandler(handler)
     assert len(logger.handlers) == 1
-    formatter = logging.Formatter("%(asctime)s  %(name)-23s"
-                                  "%(levelname)-8s %(message)s")
+    formatter = logging.Formatter("%(asctime)s - %(name)s - "
+                                  "%(levelname)s - %(message)s")
     handler.setFormatter(formatter)
     logger.setLevel(logging.DEBUG)
 

=== modified file 'magicicada/tests/test_dbusiface.py'
--- magicicada/tests/test_dbusiface.py	2011-11-19 17:14:50 +0000
+++ magicicada/tests/test_dbusiface.py	2011-12-06 00:43:23 +0000
@@ -1341,7 +1341,7 @@
         """Creating a folder finishes ok."""
         d = dict(node_id='nid', path=u'pth', subscribed='True', generation='5',
                  suggested_path=u'sgp', type='UDF', volume_id='vid')
-        self.fake_sdt_response('create_folder', (d,))
+        self.fake_sdt_response('create_folder', d)
         result = yield self.dbus.create_folder('pth')
         self.assertEqual(result.path, 'pth')
         self.assertEqual(result.suggested_path, 'sgp')
@@ -1375,7 +1375,7 @@
         """Deleting a folder finishes ok."""
         d = dict(node_id='nid', path=u'pth', subscribed='', generation='5',
                  suggested_path=u'sgp', type='UDF', volume_id='vid')
-        self.fake_sdt_response('delete_folder', (d,))
+        self.fake_sdt_response('delete_folder', d)
         result = yield self.dbus.delete_folder('vid')
         self.assertEqual(result.path, 'pth')
         self.assertEqual(result.suggested_path, 'sgp')
@@ -1409,7 +1409,7 @@
         """Subscribing a folder finishes ok."""
         d = dict(node_id='nid', path=u'pth', subscribed='', generation='5',
                  suggested_path=u'sgp', type='UDF', volume_id='vid')
-        self.fake_sdt_response('subscribe_folder', (d,))
+        self.fake_sdt_response('subscribe_folder', d)
         result = yield self.dbus.subscribe_folder('vid')
         self.assertEqual(result.path, 'pth')
         self.assertEqual(result.suggested_path, 'sgp')
@@ -1443,7 +1443,7 @@
         """Subscribing a folder finishes ok."""
         d = dict(node_id='nid', path=u'pth', subscribed='', generation='5',
                  suggested_path=u'sgp', type='UDF', volume_id='vid')
-        self.fake_sdt_response('unsubscribe_folder', (d,))
+        self.fake_sdt_response('unsubscribe_folder', d)
         result = yield self.dbus.unsubscribe_folder('vid')
         self.assertEqual(result.path, 'pth')
         self.assertEqual(result.suggested_path, 'sgp')