Merge lp:~mandel/ubuntuone-windows-installer/put-migration-together into lp:ubuntuone-windows-installer

[Manuel de la Peña]

Put all the different pieces of the migration together so that we perform the uninstall and migtration of the data if the user has the very old first beta.

Put all the different pieces of the migration together so that we perform the uninstall and migtration of the data if the user has the very old first beta.

[lp:~mandel/ubuntuone-windows-installer/put-migration-together]

94. By Manuel de la Peña on 2011-10-18

Merged with parent.

95. By Manuel de la Peña on 2011-10-18

Disable lint error.

[Diego Sarmentero (diegosarmentero)]

+1

review: Approve

[Natalia Bidart (nataliabidart)]

This is now having conflicts with trunk:

Text conflict in ubuntuone_installer/gui/qt/main/tests/test_windows.py
Text conflict in ubuntuone_installer/gui/qt/main/windows.py
Text conflict in ubuntuone_installer/gui/qt/utils/__init__.py
Text conflict in ubuntuone_installer/gui/qt/utils/windows.py
4 conflicts encountered.

review: Needs Fixing

[lp:~mandel/ubuntuone-windows-installer/put-migration-together]

96. By Manuel de la Peña on 2011-11-10

Merged with trunk, fixed conflicts.

97. By Manuel de la Peña on 2011-11-10

Fixed boken tests after merge.

98. By Manuel de la Peña on 2011-11-10

Fixed lint and pep8 issues.

99. By Manuel de la Peña on 2011-11-14

Merged with trunk and fixed conflicts.

100. By Manuel de la Peña on 2011-11-14

Added new tests for the check_credentials.

101. By Manuel de la Peña on 2011-11-14

Fixed test and pylint issues.

[Natalia Bidart (nataliabidart)]

* Can you please remove the duplication of titles for REMOVE_OLD_BETA_TITLE and REMOVE_OLD_BETA_ERROR_TITLE? the same for MIGRATE_OLD_BETA_DATA_TITLE and MIGRATE_OLD_BETA_DATA_ERROR_TITLE

* Instead of returning:

    if result == QtGui.QMessageBox.Yes:
        return True
    return False

just do:

    return result == QtGui.QMessageBox.Yes

* Typo: "if there are not credentials"

* Minor fix: in CheckCredentialsTestCase, please move the self.migrate_data_cb definition right before the calling to self.patch(utils, 'perform_old_beta_data_migration', ...), so each fake is close to each patch.

* in ubuntuone_installer/gui/qt/utils/windows.py, the definition of logger is duplicated:

line 48: logger = setup_logging('qt.utils.windows')
line 72: logger = setup_logging('qt.gui.utils.windows')

we should have only one.

* Any reason to expose the UninstallException and DataMigrationException on the utils/__init__ file? they don't seem to be used anywhere.

review: Needs Fixing

[lp:~mandel/ubuntuone-windows-installer/put-migration-together]

102. By Manuel de la Peña on 2011-11-14

Fixed code following review comments.

103. By Manuel de la Peña on 2011-11-14

Fixed failing test.

104. By Manuel de la Peña on 2011-11-14

Use the exception from the correct module.

105. By Manuel de la Peña on 2011-11-14

Fixed pep8 issue.

106. By Manuel de la Peña on 2011-11-16

Removed duplicated title.

[Natalia Bidart (nataliabidart)]

I'm getting these lint issues:

ubuntuone_installer/gui/qt/utils/tests/test_windows.py:
    559: [W0212, UpdatePathTestCase.test_get_update_path_not_frozen] Access to a protected member _get_update_path of a client class
    571: [W0212, UpdatePathTestCase.test_get_update_path_frozen] Access to a protected member _get_update_path of a client class

review: Needs Fixing

[lp:~mandel/ubuntuone-windows-installer/put-migration-together]

107. By Manuel de la Peña on 2011-11-30

Make lint happy.

108. By Manuel de la Peña on 2011-11-30

Merged with trunk.

109. By Manuel de la Peña on 2011-11-30

Merged with trunk and fixed pep8 issues after the merge.

[Natalia Bidart (nataliabidart)]

Tests won't run due to the parameter ordering, you will also need to apply this change:

=== modified file 'run-tests.bat'
--- run-tests.bat 2011-08-24 17:04:02 +0000
+++ run-tests.bat 2011-11-30 20:07:10 +0000
@@ -66,4 +66,4 @@
 "%PYTHONEXEPATH%\python.exe" setup.py build
 ECHO Running tests
 :: execute the tests with a number of ignored linux only modules
-"%PYTHONEXEPATH%\python.exe" "%PYTHONEXEPATH%\Scripts\u1trial" -i "test_linux.py" ubuntuone_installer --gui --reactor=qt4
+"%PYTHONEXEPATH%\python.exe" "%PYTHONEXEPATH%\Scripts\u1trial" -i "test_linux.py" --gui --reactor=qt4 ubuntuone_installer

review: Needs Fixing

[Manuel de la Peña (mandel)]

Sig.. sorted it out!

[lp:~mandel/ubuntuone-windows-installer/put-migration-together]

110. By Manuel de la Peña on 2011-12-01

Change the order of params to make the new ubuntuone-dev-tools happy.

111. By Manuel de la Peña on 2011-12-01

Merged with trunk and fixed conflict.

[Natalia Bidart (nataliabidart)]

Looks good now.

Please, before merging remove the exceptions added to ubuntuone_installer/gui/qt/utils/linux.py, since they are not exported nor used.

Thanks!

review: Approve

[lp:~mandel/ubuntuone-windows-installer/put-migration-together]

112. By Manuel de la Peña on 2011-12-01

Removed unused exceptions.

=== modified file 'ubuntuone_installer/gui/qt/gui.py'
--- ubuntuone_installer/gui/qt/gui.py	2011-11-17 20:05:33 +0000
+++ ubuntuone_installer/gui/qt/gui.py	2011-12-01 14:58:32 +0000
@@ -1,8 +1,4 @@
 # -*- coding: utf-8 -*-
-
-# Authors: Alejandro J. Cura <[e-mail]>
-#          Roberto Alsina <[e-mail]>
-#          Diego Sarmentero <[e-mail]>
 #
 # Copyright 2011 Canonical Ltd.
 #
@@ -97,19 +93,58 @@
 UPDATE_TITLE = _("Updates are available")
 UPDATE_SOFTWARE = _("There is a new version available. "
                     "Do you want perform an upgrade?")
+REMOVE_OLD_BETA_TITLE = _("Remove old Beta")
+REMOVE_OLD_BETA = _("The old Ubuntu One Beta is installed in your system. Do "
+                    "you want to uninstall it?")
+REMOVE_OLD_BETA_ERROR = _("There was an error uninstall the old beta. Please "
+                          "do this step manually")
+MIGRATE_OLD_BETA_DATA_TITLE = _("Migrate old data")
+MIGRATE_OLD_BETA = _("Do you want to migrate the files from the old Ubuntu "
+                     "One Beta location to the new one?")
+MIGRATE_OLD_BETA_DATA_ERROR = _("There was an error migrating your data. "
+                                "Please do this step manually.")
+
 # Invalid name logger
 # pylint: disable=C0103
 logger = setup_logging('qt.gui')
 # pylint: enable=C0103
 
 
+def _ask_user_question(title, question):
+    """Ask the user a Yes/No question."""
+    result = QtGui.QMessageBox.question(None, title, question,
+            QtGui.QMessageBox.Yes, QtGui.QMessageBox.No)
+    return result == QtGui.QMessageBox.Yes
+
+
 def user_wants_to_update():
     """Ask the user if he really wants to update the software."""
-    result = QtGui.QMessageBox.question(None, UPDATE_TITLE, UPDATE_SOFTWARE,
-            QtGui.QMessageBox.Yes, QtGui.QMessageBox.No)
-    if result == QtGui.QMessageBox.Yes:
-        return True
-    return False
+    return _ask_user_question(UPDATE_TITLE, UPDATE_SOFTWARE)
+
+
+def user_wants_to_remove_old_beta():
+    """Ask the user if he really wants to remove the old beta."""
+    return _ask_user_question(REMOVE_OLD_BETA_TITLE, REMOVE_OLD_BETA)
+
+
+def user_wants_to_migrate_data():
+    """Ask the user if he really wants to migrate the data."""
+    return _ask_user_question(MIGRATE_OLD_BETA_DATA_TITLE, MIGRATE_OLD_BETA)
+
+
+def _warn_user(title, message):
+    """Warn the user."""
+    QtGui.QMessageBox.warning(None, title, message, QtGui.QMessageBox.Ok)
+
+
+def warn_user_beta_not_removed():
+    """Let the user know that the data was not migrated."""
+    _warn_user(REMOVE_OLD_BETA_TITLE, REMOVE_OLD_BETA_ERROR)
+
+
+def warn_user_data_not_migrated():
+    """Let the user know that the data was not migrated."""
+    _warn_user(MIGRATE_OLD_BETA_DATA_TITLE, MIGRATE_OLD_BETA_DATA_ERROR)
 
 
 class LicensePage(SSOWizardPage):
 
=== modified file 'ubuntuone_installer/gui/qt/tests/test_gui.py'
--- ubuntuone_installer/gui/qt/tests/test_gui.py	2011-11-17 20:13:28 +0000
+++ ubuntuone_installer/gui/qt/tests/test_gui.py	2011-12-01 14:58:32 +0000
@@ -729,51 +729,122 @@
         self.assertTrue(self.ui.ui.existing_account_button.isDefault())
 
 
-class UpgradeQuestion(BaseTestCase):
+class MessageBoxTestCase(BaseTestCase):
     """Test the dialog that ask the user if he wants to upgrade."""
 
     @defer.inlineCallbacks
     def setUp(self):
         """Set the tests."""
-        yield super(UpgradeQuestion, self).setUp()
+        yield super(MessageBoxTestCase, self).setUp()
         self.answer = 0
         self.parent = None
         self.title = None
         self.text = None
         self.yes_button = None
         self.no_button = None
-
-        def fake_question(parent, title, text, yes_button,
-                no_button):
-            """Fake asking a question."""
-            self.parent = parent
-            self.title = title
-            self.text = text
-            self.yes_button = yes_button
-            self.no_button = no_button
-            return self.answer
-
-        self.patch(gui.QtGui.QMessageBox, 'question', fake_question)
-
-    def user_wants_to_update_true(self):
-        """Test the question."""
+        self.ok_button = None
+
+        class FakeMessageBox(object):
+            """A fake message box."""
+
+            No = 'No'
+            Yes = 'Yes'
+            Ok = 'Ok'
+
+            @classmethod
+            def question(cls, parent, title, text, yes_button,
+                              no_button):
+                """Fake asking a question."""
+                self.parent = parent
+                self.title = title
+                self.text = text
+                self.yes_button = yes_button
+                self.no_button = no_button
+                return self.answer
+
+            @classmethod
+            def warning(cls, parent, title, text, ok_button):
+                """Fake warning the user."""
+                self.parent = parent
+                self.title = title
+                self.text = text
+                self.ok_button = ok_button
+                return self.answer
+
+        self.patch(gui.QtGui, 'QMessageBox', FakeMessageBox)
+
+    def _assert_question(self, question_title, question_text):
+        """Assert the execution of a question."""
+        self.assertEqual(None, self.parent)
+        self.assertEqual(self.title, question_title)
+        self.assertEqual(self.text, question_text)
+        self.assertEqual(self.yes_button, gui.QtGui.QMessageBox.Yes)
+        self.assertEqual(self.no_button, gui.QtGui.QMessageBox.No)
+
+    def _assert_true_question(self, question_fn, question_title,
+                              question_text):
+        """Assert a positive answer to a question."""
         self.answer = gui.QtGui.QMessageBox.Yes
-        self.assertTrue(gui.user_wants_to_update(), 'User wants to update.')
-        # lets assert the parameters
-        self.assertTrue(None, self.parent)
-        self.assertTrue(self.title, gui.UPDATE_TITLE)
-        self.assertTrue(self.text, gui.UPDATE_SOFTWARE)
-        self.assertTrue(self.yes_button, gui.QtGui.QMessageBox.Yes)
-        self.assertTrue(self.no_button, gui.QtGui.QMessageBox.No)
+        self.assertTrue(question_fn())
+        self._assert_question(question_title, question_text)
 
-    def user_wants_to_update_false(self):
-        """Test the question."""
+    def _assert_false_question(self, question_fn, question_title,
+                               question_text):
+        """Assert a negative answer to a question."""
         self.answer = gui.QtGui.QMessageBox.No
-        msg = 'user_wants_to_update must return False when the answer is No.'
-        self.assertFalse(gui.user_wants_to_update(), msg)
-        # lets assert the parameters
-        self.assertTrue(None, self.parent)
-        self.assertTrue(self.title, gui.UPDATE_TITLE)
-        self.assertTrue(self.text, gui.UPDATE_SOFTWARE)
-        self.assertTrue(self.yes_button, gui.QtGui.QMessageBox.Yes)
-        self.assertTrue(self.no_button, gui.QtGui.QMessageBox.No)
+        self.assertFalse(question_fn())
+        self._assert_question(question_title, question_text)
+
+    def test_user_wants_to_update_true(self):
+        """Test the question."""
+        self._assert_true_question(gui.user_wants_to_update, gui.UPDATE_TITLE,
+                                   gui.UPDATE_SOFTWARE)
+
+    def test_user_wants_to_update_false(self):
+        """Test the question."""
+        self._assert_false_question(gui.user_wants_to_update, gui.UPDATE_TITLE,
+                                    gui.UPDATE_SOFTWARE)
+
+    def test_user_wants_to_remove_true(self):
+        """Test the question."""
+        self._assert_true_question(gui.user_wants_to_remove_old_beta,
+                                   gui.REMOVE_OLD_BETA_TITLE,
+                                   gui.REMOVE_OLD_BETA)
+
+    def test_user_wants_to_remove_false(self):
+        """Test the question."""
+        self._assert_false_question(gui.user_wants_to_remove_old_beta,
+                                    gui.REMOVE_OLD_BETA_TITLE,
+                                    gui.REMOVE_OLD_BETA)
+
+    def test_user_wants_to_migrate_true(self):
+        """Test the question."""
+        self._assert_true_question(gui.user_wants_to_migrate_data,
+                                   gui.MIGRATE_OLD_BETA_DATA_TITLE,
+                                   gui.MIGRATE_OLD_BETA)
+
+    def test_user_wants_to_migrate_false(self):
+        """Test the question."""
+        self._assert_false_question(gui.user_wants_to_migrate_data,
+                                    gui.MIGRATE_OLD_BETA_DATA_TITLE,
+                                    gui.MIGRATE_OLD_BETA)
+
+    def _assert_warn_user(self, warn_fn, title, text):
+        """Assert waning a user."""
+        self.assertEqual(None, warn_fn())
+        self.assertEqual(None, self.parent)
+        self.assertEqual(self.title, title)
+        self.assertEqual(self.text, text)
+        self.assertEqual(self.ok_button, gui.QtGui.QMessageBox.Ok)
+
+    def test_warn_user_data_migration(self):
+        """Test warning the user."""
+        self._assert_warn_user(gui.warn_user_data_not_migrated,
+                               gui.MIGRATE_OLD_BETA_DATA_TITLE,
+                               gui.MIGRATE_OLD_BETA_DATA_ERROR)
+
+    def test_warn_user_old_app_removal(self):
+        """Test warning the user."""
+        self._assert_warn_user(gui.warn_user_beta_not_removed,
+                               gui.REMOVE_OLD_BETA_TITLE,
+                               gui.REMOVE_OLD_BETA_ERROR)
 
=== modified file 'ubuntuone_installer/gui/qt/utils/__init__.py'
--- ubuntuone_installer/gui/qt/utils/__init__.py	2011-11-11 19:27:39 +0000
+++ ubuntuone_installer/gui/qt/utils/__init__.py	2011-12-01 14:58:32 +0000
@@ -42,6 +42,7 @@
     uninstall_old_beta = windows.uninstall_old_beta
     migrate_old_data = windows.migrate_old_data
     check_updates = windows.check_updates
+    perform_old_beta_data_migration = windows.perform_old_beta_data_migration
 else:
     from ubuntuone_installer.gui.qt.utils import linux
     uninstall_application = linux.uninstall_application
@@ -54,6 +55,7 @@
     uninstall_old_beta = lambda *args, **kwargs: None
     migrate_old_data = lambda *args, **kwargs: None
     check_updates = lambda *args, **kwargs: None
+    perform_old_beta_data_migration = lambda *args, **kwargs: None
 # pylint: enable=C0103
 
 
@@ -76,6 +78,10 @@
             logger.info('Updates checked, stopping.')
             stop_cb()
         else:  # No credentials
+            # if there are no credentials it usually means that we
+            # are installing the application, before we do anything
+            # lets check if the old beta is present
+            yield perform_old_beta_data_migration(gui)
             logger.info('Got empty credentials, starting wizard app.')
             window = gui.MainWindow(close_callback=stop_cb,
                                     installing=installing)
 
=== modified file 'ubuntuone_installer/gui/qt/utils/tests/test_common.py'
--- ubuntuone_installer/gui/qt/utils/tests/test_common.py	2011-11-11 19:28:10 +0000
+++ ubuntuone_installer/gui/qt/utils/tests/test_common.py	2011-12-01 14:58:32 +0000
@@ -69,11 +69,19 @@
 
         self.start_cp = lambda with_icon: \
                         self.called.extend(('start_cp', with_icon))
+
         self.patch(utils, 'start_control_panel', self.start_cp)
 
         self.check_updates = lambda _: self.called.append('check_updates')
+
         self.patch(utils, 'check_updates', self.check_updates)
 
+        self.migrate_data_cb = lambda gui: \
+                        self.called.extend(('migrate_data_cb', gui))
+
+        self.patch(utils, 'perform_old_beta_data_migration',
+           self.migrate_data_cb)
+
         self.patch(gui, 'MainWindow', FakeWindow)
 
     @defer.inlineCallbacks
@@ -110,3 +118,4 @@
         self.assertTrue(FakeWindow.visible)
         msgs = ('Got empty credentials', 'starting wizard')
         self.assertTrue(self.memento.check_info(*msgs))
+        self.assertEqual(self.called, ['migrate_data_cb', gui])
 
=== modified file 'ubuntuone_installer/gui/qt/utils/tests/test_windows.py'
--- ubuntuone_installer/gui/qt/utils/tests/test_windows.py	2011-11-16 15:42:44 +0000
+++ ubuntuone_installer/gui/qt/utils/tests/test_windows.py	2011-12-01 14:58:32 +0000
@@ -37,6 +37,9 @@
 from ubuntuone_installer.gui.qt import utils
 from ubuntuone_installer.gui.qt.tests import BaseTestCase
 
+# we want to test private methods
+# pylint:disable=W0212
+
 
 class UninstallerTestCase(BaseTestCase):
 
@@ -395,7 +398,7 @@
         self.patch(utils.windows, 'deferToThread',
                    fake_msi_install_product)
         self.assertFailure(utils.uninstall_old_beta(),
-                           utils.windows.MsiException)
+                           utils.windows.UninstallException)
         self.assertEqual(self.called['property_uid'],
                          utils.windows.OLD_BETA_UID_KEY)
         self.assertEqual(self.called['property_name'], u'LocalPackage')
@@ -416,7 +419,7 @@
         self.patch(utils.windows, 'get_property_for_product',
                    fake_get_property_for_product)
         self.assertFailure(utils.uninstall_old_beta(),
-                           utils.windows.MsiException)
+                           utils.windows.UninstallException)
         self.assertEqual(self.called['uid'], utils.windows.OLD_BETA_UID_KEY)
         self.assertEqual(self.called['property_name'], u'LocalPackage')
 
 
=== modified file 'ubuntuone_installer/gui/qt/utils/windows.py'
--- ubuntuone_installer/gui/qt/utils/windows.py	2011-11-16 15:42:44 +0000
+++ ubuntuone_installer/gui/qt/utils/windows.py	2011-12-01 14:58:32 +0000
@@ -69,7 +69,8 @@
 LONG_PATH_PREFIX = u'\\\\?\\'
 AUTO_UPDATE_EXE = 'autoupdate-windows.exe'
 
-class MsiException(Exception):
+
+class UninstallException(Exception):
     """Raised when there are msi issues."""
 
 
@@ -261,10 +262,10 @@
         result = yield deferToThread(ctypes.windll.msi.MsiInstallProductW,
                                      uninstall_path, command_line)
         if result != ERROR_SUCCESS:
-            raise MsiException('Could not remove old beta.')
+            raise UninstallException('Could not remove old beta.')
     else:
         # the local file is missing so we cannot un install it :(
-        raise MsiException('Could not remove old beta.')
+        raise UninstallException('Could not remove old beta.')
 
 
 def _append_long_path(path):
@@ -306,3 +307,26 @@
             new_full_path = os.path.join(new_location, path)
             yield deferToThread(shutil.move, old_full_path,
                                 new_full_path)
+
+
[e-mail]
+def perform_old_beta_data_migration(gui):
+    """Perform the migration from the old beta."""
+    if is_old_beta_installed() and \
+            gui.user_wants_to_remove_old_beta():
+        logger.debug('Old beta is installed and user wants to remove it.')
+        try:
+            # uninstall the msi and migrate data
+            yield uninstall_old_beta()
+            # ask the user if he wants to move his data
+            if gui.user_wants_to_migrate_data():
+                yield migrate_old_data()
+        except UninstallException:
+            logger.error('Could not uninstall msi: %s')
+            gui.warn_user_beta_not_removed()
+        except DataMigrationException:
+            # we did uninstall the msi \o/ but failed in the data migration.
+            # This is not a big deal, lets just tell the user that he has to
+            # manually do it
+            logger.error('Could not migrate data: %s')
+            gui.warn_user_data_not_migrated()