Merge lp:~nataliabidart/ubuntuone-control-panel/handle-credentials-error into lp:ubuntuone-control-panel

[Natalia Bidart]

- Handle errors from backend on the signin wizard page (LP: #945078).
- Avoid the 'show/hide details' button to grow when focused (LP: #961348).

[Roberto Alsina (ralsina)]

+1 I suggested connecting box.finished to box.deleteLater to avoid leaking it, but it's +1 with or without that.

review: Approve

[lp:~nataliabidart/ubuntuone-control-panel/handle-credentials-error]

303. By Natalia Bidart on 2012-03-27

- Also deleting the box that shows errors messages after is used.

[Diego Sarmentero (diegosarmentero)]

+1

review: Approve

=== modified file 'data/qt/ubuntuone.qss'
--- data/qt/ubuntuone.qss	2012-03-22 20:28:48 +0000
+++ data/qt/ubuntuone.qss	2012-03-27 12:55:22 +0000
@@ -365,3 +365,12 @@
     background: #fcece7;
     color: black;
 }
+
+
+QMessageBox > QWidget > QPushButton:focus {
+    /* use the same padding from the original QPushButton:focus setting */
+    /* hack to make the mild-orange focused box dissapear */
+    padding-top: 5px;
+    padding-bottom: 5px;
+    /* end of hack */
+}

=== modified file 'ubuntuone/controlpanel/gui/qt/__init__.py'
--- ubuntuone/controlpanel/gui/qt/__init__.py	2012-03-02 13:53:24 +0000
+++ ubuntuone/controlpanel/gui/qt/__init__.py	2012-03-27 12:55:22 +0000
@@ -70,6 +70,12 @@
     return icon
 
 
+# Invalid name "box"
+# pylint: disable=C0103
+box = None
+# pylint: enable=C0103
+
+
 def handle_errors(error_handler=None, logger=None):
     """Decorator to handle errors when calling a function.
 
@@ -107,13 +113,35 @@
             else:
                 msgs = [e.__class__.__name__] + map(repr, e.args)
             msg = '\n'.join(msgs)
+
+            # hack to avoid the QMessageBox being gargabe-collected too soon
+            # Using the global statement, pylint: disable=W0603
+            global box
+            # pylint: enable=W0603
             box = QtGui.QMessageBox(QtGui.QMessageBox.Critical,
-                                    GENERAL_ERROR_TITLE, GENERAL_ERROR_MSG,
-                                    QtGui.QMessageBox.Close,
-                                    parent=None)
+                    GENERAL_ERROR_TITLE, GENERAL_ERROR_MSG,
+                    QtGui.QMessageBox.Close, parent=None)
+            box.setDefaultButton(QtGui.QMessageBox.Close)
             box.setDetailedText(msg)
-            box.setDefaultButton(QtGui.QMessageBox.Close)
-            box.exec_()
+
+            # can not exec_() on the QMessageBox since that will block the Qt
+            # mainloop
+
+            d = defer.Deferred()
+
+            def box_finished(result):
+                """The QMessageBox finished."""
+                logger.info('The warning dialog was shown and also closed '
+                            '(message was %r).', msg)
+                box.finished.disconnect(box_finished)
+                box.hide()
+                box.deleteLater()
+                d.callback(result)
+
+            box.finished.connect(box_finished)
+            box.show()
+
+            yield d
 
         return inner
 

=== modified file 'ubuntuone/controlpanel/gui/qt/tests/__init__.py'
--- ubuntuone/controlpanel/gui/qt/tests/__init__.py	2012-03-21 21:13:13 +0000
+++ ubuntuone/controlpanel/gui/qt/tests/__init__.py	2012-03-27 12:55:22 +0000
@@ -187,17 +187,16 @@
         return inner
 
 
-class FakedDialog(object):
+class FakedDialog(QtGui.QMessageBox):
     """Fake a confirmation dialog."""
 
-    Close = object()
-    Critical = object()
     properties = response = args = kwargs = None
 
     def __init__(self, *a, **kw):
+        super(FakedDialog, self).__init__(*a, **kw)
         FakedDialog.args = a
         FakedDialog.kwargs = kw
-        FakedDialog.properties = {'exec_': 0}
+        FakedDialog.properties = {'shown': 0}
 
     @classmethod
     def reset(cls, *a, **kw):
@@ -216,17 +215,24 @@
 
     def setDetailedText(self, text):
         """Fake the setDetailedText."""
+        super(FakedDialog, self).setDetailedText(text)
         FakedDialog.properties['detailed_text'] = text
 
     def setDefaultButton(self, button):
         """Fake the setDefaultButton."""
+        super(FakedDialog, self).setDefaultButton(button)
         FakedDialog.properties['default_button'] = button
 
     # pylint: enable=C0103
 
+    def show(self):
+        """Fake show."""
+        FakedDialog.properties['shown'] += 1
+        self.finished.emit(0)
+
     def exec_(self):
         """Fake exec_."""
-        FakedDialog.properties['exec_'] += 1
+        FakedDialog.properties['shown'] += 1
 
 
 class FakedFileDialog(object):

=== modified file 'ubuntuone/controlpanel/gui/qt/tests/test_common.py'
--- ubuntuone/controlpanel/gui/qt/tests/test_common.py	2011-10-05 16:35:59 +0000
+++ ubuntuone/controlpanel/gui/qt/tests/test_common.py	2012-03-27 12:55:22 +0000
@@ -132,7 +132,8 @@
         yield self.decorate_me()
 
         logged = self.memento.check_exception(self.failure.__class__, msg)
-        recs = '\n'.join(rec.exc_text for rec in self.memento.records)
+        recs = '\n'.join(rec.exc_text for rec in self.memento.records
+                         if rec.exc_text is not None)
         self.assertTrue(logged, 'Exception must be logged, got:\n%s' % recs)
 
     @defer.inlineCallbacks
@@ -147,7 +148,7 @@
                 QtGui.QMessageBox.Close)
         self.assertEqual(FakedDialog.args, args)
         self.assertEqual(FakedDialog.kwargs, {'parent': None})
-        self.assertEqual(FakedDialog.properties['exec_'], 1)
+        self.assertEqual(FakedDialog.properties['shown'], 1)
 
     @defer.inlineCallbacks
     def test_show_error_message_default_button(self):

=== modified file 'ubuntuone/controlpanel/gui/qt/tests/test_controlpanel.py'
--- ubuntuone/controlpanel/gui/qt/tests/test_controlpanel.py	2012-03-21 20:27:38 +0000
+++ ubuntuone/controlpanel/gui/qt/tests/test_controlpanel.py	2012-03-27 12:55:22 +0000
@@ -1,8 +1,6 @@
 # -*- coding: utf-8 -*-
-
-# Authors: Natalia B Bidart <[e-mail]>
 #
-# Copyright 2011 Canonical Ltd.
+# Copyright 2011-2012 Canonical Ltd.
 #
 # This program is free software: you can redistribute it and/or modify it
 # under the terms of the GNU General Public License version 3, as published

=== modified file 'ubuntuone/controlpanel/gui/qt/tests/test_wizard.py'
--- ubuntuone/controlpanel/gui/qt/tests/test_wizard.py	2012-03-21 20:27:38 +0000
+++ ubuntuone/controlpanel/gui/qt/tests/test_wizard.py	2012-03-27 12:55:22 +0000
@@ -89,7 +89,7 @@
         # Finish button is visible (and enabled if isComplete() returns true).
 
         # After calling setFinalPage(false), isFinalPage() returns true if
-        #nextId() returns -1; otherwise, it returns false.
+        # nextId() returns -1; otherwise, it returns false.
 
         self.patch(self.ui, 'nextId', lambda *a: 0)
         self.assertFalse(self.ui.isFinalPage())
@@ -385,6 +385,27 @@
         self.assertTrue(self.ui.signin_page.isEnabled())
         self.assertFalse(self._called)
 
+    @defer.inlineCallbacks
+    def test_with_error(self):
+        """Wizard is enabled after an error was handled."""
+        self.ui.currentIdChanged.connect(self._set_called)
+        d = defer.fail(ValueError())
+
+        def check():
+            """Confirm the ui is disabled while processing."""
+            self.assertFalse(self.ui.signin_page.isEnabled())
+            return d
+
+        self.patch(self.ui.backend, self.method, check)
+        button = getattr(self.ui.signin_page.panel.ui,
+                         '%s_button' % self.method)
+        button.click()
+
+        yield d
+
+        self.assertTrue(self.ui.signin_page.isEnabled())
+        self.assertFalse(self._called)  # the wizard page was not changed
+
 
 class UbuntuOneWizardRegisterTestCase(UbuntuOneWizardLoginTestCase):
     """Test the register through the wizard."""

=== modified file 'ubuntuone/controlpanel/gui/qt/wizard.py'
--- ubuntuone/controlpanel/gui/qt/wizard.py	2012-03-23 20:39:35 +0000
+++ ubuntuone/controlpanel/gui/qt/wizard.py	2012-03-27 12:55:22 +0000
@@ -44,6 +44,7 @@
     LICENSE_LINK,
     UBUNTUONE_LINK,
 )
+from ubuntuone.controlpanel.gui.qt import handle_errors
 from ubuntuone.controlpanel.gui.qt.folders import (
     RemoteFoldersPanel,
     LocalFoldersPanel,
@@ -300,22 +301,28 @@
             self.next()
 
     @QtCore.pyqtSlot()
+    @handle_errors(logger=logger)
     @defer.inlineCallbacks
     def login(self):
         """Show the login dialog."""
         self.setEnabled(False)
-        credentials = yield self.backend.login()
-        self._process_credentials(credentials)
-        self.setEnabled(True)
+        try:
+            credentials = yield self.backend.login()
+            self._process_credentials(credentials)
+        finally:
+            self.setEnabled(True)
 
     @QtCore.pyqtSlot()
+    @handle_errors(logger=logger)
     @defer.inlineCallbacks
     def register(self):
         """Show the register dialog."""
         self.setEnabled(False)
-        credentials = yield self.backend.register()
-        self._process_credentials(credentials)
-        self.setEnabled(True)
+        try:
+            credentials = yield self.backend.register()
+            self._process_credentials(credentials)
+        finally:
+            self.setEnabled(True)
 
     @QtCore.pyqtSlot()
     def check_settings(self):