Merge lp:~mikemc/ubuntuone-control-panel/remote-folders-fix into lp:ubuntuone-control-panel

[Mike McCracken]

- Connect files service in wizard to enable display of remote folders. (LP: #978043)

- Connect files service in wizard to enable display of remote folders. (LP: #978043)

Summary:

The remote folders panel requires a syncdaemon with an active actionqueue to process the ListVolumes action, in order to display the remote folders. Connecting the files service makes that happen.

To test:

- remove your credentials, kill syncdaemon
(OR start on a new user account)
- open control panel, log in with your existing account
- does the first wizard page hang with a loading overlay or show remote folders?
(It should show remote folders)

Gritty Details:

CloudToComputer page uses folders.py RemoteFoldersPanel, which calls
backend.py volumes_info() with refresh=True. Setting refresh=True
means it'll call sd_client.refresh_volumes(), which eventually calls
interaction_interfaces.py refresh_volumes(), which calls
volume_manager.py refresh_volumes(), which calls action_queue.py
list_volumes(), which queues a ListVolumes class, which gets stuck
because the queue is inactive, and won't get re-enabled until we are
done with the wizard, which triggers on_wizard_finished() in
controlpanel.py, which calls on_credentials_found() and that'll start
the queue, but by then it's too late.

The fix is to call backend.connect_files() in wizard.py
_process_credentials(), so that the queue is active when we load the
CloudToComputerPage. To work, this has to actually connect, so it
ignores the autoconnect setting, which I assert is OK because we
probably don't get to this page with non-default settings, and
autoconnect is on by default.

So, how does the folders tab in the main management UI work when
autoconnect is False? If the main control panel found creds, then
it'll check autoconnect and if it's false, it doesn't call
backend.connect_files(). It just goes ahead and shows the folders tab
However, the folders tab calls backend.volumes_info() with
refresh=False, so none of that stuff above happens, and it doesn't
matter what state the queue is in (hence it doesn't matter if
autoconnect is enabled or not, the folders tab will still work).

[lp:~mikemc/ubuntuone-control-panel/remote-folders-fix]

358. By Mike McCracken on 2012-09-24

merge with trunk

359. By Mike McCracken on 2012-09-28

Test calling connect_files after receiving credentials.

[Roberto Alsina (ralsina)]

review: Approve

[Diego Sarmentero (diegosarmentero)]

+1

review: Approve

[lp:~mikemc/ubuntuone-control-panel/remote-folders-fix]

360. By Mike McCracken on 2012-10-17

merge with trunk and resolve conflict

[Roberto Alsina (ralsina)]

+1

review: Approve (check conflict resolution)

[lp:~mikemc/ubuntuone-control-panel/remote-folders-fix]

361. By Mike McCracken on 2012-10-18

merge with trunk and resolve conflict in controlpanel.py

362. By Mike McCracken on 2012-11-13

merge with current trunk

[Brian Curtin (brian.curtin)]

Approved on Windows. Code looks good, tests pass, and IRL on Windows looks good.

review: Approve

=== modified file 'ubuntuone/controlpanel/gui/qt/controlpanel.py'
--- ubuntuone/controlpanel/gui/qt/controlpanel.py	2012-10-16 11:31:33 +0000
+++ ubuntuone/controlpanel/gui/qt/controlpanel.py	2012-11-13 10:10:24 +0000
@@ -100,7 +100,7 @@
 
     @log_call(logger.debug)
     def on_credentials_not_found(self):
-        """Credentials are not found or were removed."""
+        """Credentials not found / were removed. Log in, then show wizard."""
         self.ui.wizard.restart()
         self.ui.switcher.setCurrentWidget(self.ui.wizard)
         # By design request, when the user logs in, he has to end
@@ -113,10 +113,14 @@
     @defer.inlineCallbacks
     @log_call(logger.debug)
     def on_credentials_found(self):
-        """Credentials are not found or were removed."""
+        """Credentials are found on first try, connect and show mgmt UI."""
+        self.connect_file_sync()
+        yield self.show_management_ui()
+
+    @defer.inlineCallbacks
+    def show_management_ui(self):
+        """Show mgmt UI. File sync is already connected if desired."""
         self.ui.switcher.setCurrentWidget(self.ui.management)
-        self.connect_file_sync()
-
         info = yield self.backend.account_info()
         self.process_info(info)
         self.is_processing = False
@@ -182,7 +186,8 @@
     @log_call(logger.info)
     def on_wizard_finished(self, status):
         """Move to controlpanel if wizard ended successfully."""
-        self.on_credentials_found()
+        # NOTE: the wizard has connected the files service already
+        self.show_management_ui()
 
     @log_call(logger.info)
     def start_from_license(self):
 
=== modified file 'ubuntuone/controlpanel/gui/qt/tests/test_controlpanel.py'
--- ubuntuone/controlpanel/gui/qt/tests/test_controlpanel.py	2012-10-08 18:11:55 +0000
+++ ubuntuone/controlpanel/gui/qt/tests/test_controlpanel.py	2012-11-13 10:10:24 +0000
@@ -91,10 +91,9 @@
         self.assertEqual(self._called, ((), {}))
 
     def test_on_credentials_found(self):
-        """The management panel is shown."""
+        """File sync is connected and the management panel is shown."""
         self.patch(self.ui, 'connect_file_sync', self._set_called)
-        self.ui.on_credentials_not_found()
-        self.ui.on_credentials_found()
+        yield self.ui.on_credentials_found()
 
         self.assertIs(self.ui.ui.switcher.currentWidget(),
                       self.ui.ui.management)
@@ -154,7 +153,7 @@
 
     def test_on_wizard_finished(self):
         """When the wizard is finished, the management panel is shown."""
-        self.patch(self.ui, 'on_credentials_found', self._set_called)
+        self.patch(self.ui, 'show_management_ui', self._set_called)
         self.ui.ui.wizard.finished.emit(1)
 
         self.assertEqual(self._called, ((), {}))
 
=== modified file 'ubuntuone/controlpanel/gui/qt/tests/test_wizard.py'
--- ubuntuone/controlpanel/gui/qt/tests/test_wizard.py	2012-03-29 19:05:02 +0000
+++ ubuntuone/controlpanel/gui/qt/tests/test_wizard.py	2012-11-13 10:10:24 +0000
@@ -296,7 +296,7 @@
 
 
 class UbuntuOneWizardSettingsTestCase(UbuntuOneWizardSignInTestCase):
-    """Test the CloudToComputerPage wizard page."""
+    """Test the Settings wizard page."""
 
     buttons = {'BackButton': (None, 'currentIdChanged', (1,))}
     page_name = 'settings'
@@ -304,7 +304,7 @@
 
 
 class UbuntuOneWizardComputerToCloudTestCase(UbuntuOneWizardSignInTestCase):
-    """Test the CloudToComputerPage wizard page."""
+    """Test the ComputerToCloudPage wizard page."""
 
     buttons = {
         'FinishButton': (None, 'finished', (gui.QtGui.QDialog.Accepted,)),
@@ -354,6 +354,17 @@
     method = 'login'
 
     @defer.inlineCallbacks
+    def setUp(self):
+        yield super(UbuntuOneWizardLoginTestCase, self).setUp()
+        self.connect_files_called = False
+
+        def fake_connect_files():
+            self.connect_files_called = True
+
+        self.patch(self.ui.backend, 'connect_files',
+                   fake_connect_files)
+
+    @defer.inlineCallbacks
     def test_with_credentials(self):
         """Wizard is done when credentials were retrieved."""
         self.ui.currentIdChanged.connect(self._set_called)
@@ -370,7 +381,7 @@
         button.click()
 
         yield d
-
+        self.assertEqual(True, self.connect_files_called)
         self.assertTrue(self.ui.signin_page.isEnabled())
         expected_next_id = self.ui.pages[self.ui.cloud_folders_page]
         self.assertEqual(self._called, ((expected_next_id,), {}))
@@ -392,7 +403,7 @@
         button.click()
 
         yield d
-
+        self.assertEqual(False, self.connect_files_called)
         self.assertTrue(self.ui.signin_page.isEnabled())
         self.assertFalse(self._called)
 
@@ -413,7 +424,7 @@
         button.click()
 
         yield d
-
+        self.assertEqual(False, self.connect_files_called)
         self.assertTrue(self.ui.signin_page.isEnabled())
         self.assertFalse(self._called)  # the wizard page was not changed
 
 
=== modified file 'ubuntuone/controlpanel/gui/qt/wizard.py'
--- ubuntuone/controlpanel/gui/qt/wizard.py	2012-03-29 19:05:02 +0000
+++ ubuntuone/controlpanel/gui/qt/wizard.py	2012-11-13 10:10:24 +0000
@@ -298,6 +298,10 @@
     def _process_credentials(self, credentials=None):
         """Confirm which is the next step after analyzing 'credentials'."""
         if credentials:
+            # NOTE: we don't check the autoconnect key here because we
+            # do need the connection for the initial remote volumes
+            # list, and it is on by default anyway.
+            self.backend.connect_files()
             self.next()
 
     @QtCore.pyqtSlot()