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

Proposed by Manuel de la Peña
Status: Merged
Approved by: Manuel de la Peña
Approved revision: 112
Merged at revision: 94
Proposed branch: lp:~mandel/ubuntuone-windows-installer/put-migration-together
Merge into: lp:ubuntuone-windows-installer
Prerequisite: lp:~mandel/ubuntuone-windows-installer/migrate-data
Diff against target: 383 lines (+196/-48)
6 files modified
ubuntuone_installer/gui/qt/gui.py (+44/-9)
ubuntuone_installer/gui/qt/tests/test_gui.py (+105/-34)
ubuntuone_installer/gui/qt/utils/__init__.py (+6/-0)
ubuntuone_installer/gui/qt/utils/tests/test_common.py (+9/-0)
ubuntuone_installer/gui/qt/utils/tests/test_windows.py (+5/-2)
ubuntuone_installer/gui/qt/utils/windows.py (+27/-3)
To merge this branch: bzr merge lp:~mandel/ubuntuone-windows-installer/put-migration-together
Reviewer Review Type Date Requested Status
Natalia Bidart (community) Approve
Diego Sarmentero (community) Approve
Review via email: mp+78995@code.launchpad.net

Commit message

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.

Description of the change

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.

To post a comment you must log in.
94. By Manuel de la Peña

Merged with parent.

95. By Manuel de la Peña

Disable lint error.

Revision history for this message
Diego Sarmentero (diegosarmentero) wrote :

+1

review: Approve
Revision history for this message
Natalia Bidart (nataliabidart) wrote :

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
96. By Manuel de la Peña

Merged with trunk, fixed conflicts.

97. By Manuel de la Peña

Fixed boken tests after merge.

98. By Manuel de la Peña

Fixed lint and pep8 issues.

99. By Manuel de la Peña

Merged with trunk and fixed conflicts.

100. By Manuel de la Peña

Added new tests for the check_credentials.

101. By Manuel de la Peña

Fixed test and pylint issues.

Revision history for this message
Natalia Bidart (nataliabidart) wrote :

* 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
102. By Manuel de la Peña

Fixed code following review comments.

103. By Manuel de la Peña

Fixed failing test.

104. By Manuel de la Peña

Use the exception from the correct module.

105. By Manuel de la Peña

Fixed pep8 issue.

106. By Manuel de la Peña

Removed duplicated title.

Revision history for this message
Natalia Bidart (nataliabidart) wrote :

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
107. By Manuel de la Peña

Make lint happy.

108. By Manuel de la Peña

Merged with trunk.

109. By Manuel de la Peña

Merged with trunk and fixed pep8 issues after the merge.

Revision history for this message
Natalia Bidart (nataliabidart) wrote :

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
Revision history for this message
Manuel de la Peña (mandel) wrote :

Sig.. sorted it out!

110. By Manuel de la Peña

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

111. By Manuel de la Peña

Merged with trunk and fixed conflict.

Revision history for this message
Natalia Bidart (nataliabidart) wrote :

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
112. By Manuel de la Peña

Removed unused exceptions.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'ubuntuone_installer/gui/qt/gui.py'
2--- ubuntuone_installer/gui/qt/gui.py 2011-11-17 20:05:33 +0000
3+++ ubuntuone_installer/gui/qt/gui.py 2011-12-01 14:58:32 +0000
4@@ -1,8 +1,4 @@
5 # -*- coding: utf-8 -*-
6-
7-# Authors: Alejandro J. Cura <alecu@canonical.com>
8-# Roberto Alsina <roberto.alsina@canonical.com>
9-# Diego Sarmentero <diego.sarmentero@canonical.com>
10 #
11 # Copyright 2011 Canonical Ltd.
12 #
13@@ -97,19 +93,58 @@
14 UPDATE_TITLE = _("Updates are available")
15 UPDATE_SOFTWARE = _("There is a new version available. "
16 "Do you want perform an upgrade?")
17+REMOVE_OLD_BETA_TITLE = _("Remove old Beta")
18+REMOVE_OLD_BETA = _("The old Ubuntu One Beta is installed in your system. Do "
19+ "you want to uninstall it?")
20+REMOVE_OLD_BETA_ERROR = _("There was an error uninstall the old beta. Please "
21+ "do this step manually")
22+MIGRATE_OLD_BETA_DATA_TITLE = _("Migrate old data")
23+MIGRATE_OLD_BETA = _("Do you want to migrate the files from the old Ubuntu "
24+ "One Beta location to the new one?")
25+MIGRATE_OLD_BETA_DATA_ERROR = _("There was an error migrating your data. "
26+ "Please do this step manually.")
27+
28 # Invalid name logger
29 # pylint: disable=C0103
30 logger = setup_logging('qt.gui')
31 # pylint: enable=C0103
32
33
34+def _ask_user_question(title, question):
35+ """Ask the user a Yes/No question."""
36+ result = QtGui.QMessageBox.question(None, title, question,
37+ QtGui.QMessageBox.Yes, QtGui.QMessageBox.No)
38+ return result == QtGui.QMessageBox.Yes
39+
40+
41 def user_wants_to_update():
42 """Ask the user if he really wants to update the software."""
43- result = QtGui.QMessageBox.question(None, UPDATE_TITLE, UPDATE_SOFTWARE,
44- QtGui.QMessageBox.Yes, QtGui.QMessageBox.No)
45- if result == QtGui.QMessageBox.Yes:
46- return True
47- return False
48+ return _ask_user_question(UPDATE_TITLE, UPDATE_SOFTWARE)
49+
50+
51+def user_wants_to_remove_old_beta():
52+ """Ask the user if he really wants to remove the old beta."""
53+ return _ask_user_question(REMOVE_OLD_BETA_TITLE, REMOVE_OLD_BETA)
54+
55+
56+def user_wants_to_migrate_data():
57+ """Ask the user if he really wants to migrate the data."""
58+ return _ask_user_question(MIGRATE_OLD_BETA_DATA_TITLE, MIGRATE_OLD_BETA)
59+
60+
61+def _warn_user(title, message):
62+ """Warn the user."""
63+ QtGui.QMessageBox.warning(None, title, message, QtGui.QMessageBox.Ok)
64+
65+
66+def warn_user_beta_not_removed():
67+ """Let the user know that the data was not migrated."""
68+ _warn_user(REMOVE_OLD_BETA_TITLE, REMOVE_OLD_BETA_ERROR)
69+
70+
71+def warn_user_data_not_migrated():
72+ """Let the user know that the data was not migrated."""
73+ _warn_user(MIGRATE_OLD_BETA_DATA_TITLE, MIGRATE_OLD_BETA_DATA_ERROR)
74
75
76 class LicensePage(SSOWizardPage):
77
78=== modified file 'ubuntuone_installer/gui/qt/tests/test_gui.py'
79--- ubuntuone_installer/gui/qt/tests/test_gui.py 2011-11-17 20:13:28 +0000
80+++ ubuntuone_installer/gui/qt/tests/test_gui.py 2011-12-01 14:58:32 +0000
81@@ -729,51 +729,122 @@
82 self.assertTrue(self.ui.ui.existing_account_button.isDefault())
83
84
85-class UpgradeQuestion(BaseTestCase):
86+class MessageBoxTestCase(BaseTestCase):
87 """Test the dialog that ask the user if he wants to upgrade."""
88
89 @defer.inlineCallbacks
90 def setUp(self):
91 """Set the tests."""
92- yield super(UpgradeQuestion, self).setUp()
93+ yield super(MessageBoxTestCase, self).setUp()
94 self.answer = 0
95 self.parent = None
96 self.title = None
97 self.text = None
98 self.yes_button = None
99 self.no_button = None
100-
101- def fake_question(parent, title, text, yes_button,
102- no_button):
103- """Fake asking a question."""
104- self.parent = parent
105- self.title = title
106- self.text = text
107- self.yes_button = yes_button
108- self.no_button = no_button
109- return self.answer
110-
111- self.patch(gui.QtGui.QMessageBox, 'question', fake_question)
112-
113- def user_wants_to_update_true(self):
114- """Test the question."""
115+ self.ok_button = None
116+
117+ class FakeMessageBox(object):
118+ """A fake message box."""
119+
120+ No = 'No'
121+ Yes = 'Yes'
122+ Ok = 'Ok'
123+
124+ @classmethod
125+ def question(cls, parent, title, text, yes_button,
126+ no_button):
127+ """Fake asking a question."""
128+ self.parent = parent
129+ self.title = title
130+ self.text = text
131+ self.yes_button = yes_button
132+ self.no_button = no_button
133+ return self.answer
134+
135+ @classmethod
136+ def warning(cls, parent, title, text, ok_button):
137+ """Fake warning the user."""
138+ self.parent = parent
139+ self.title = title
140+ self.text = text
141+ self.ok_button = ok_button
142+ return self.answer
143+
144+ self.patch(gui.QtGui, 'QMessageBox', FakeMessageBox)
145+
146+ def _assert_question(self, question_title, question_text):
147+ """Assert the execution of a question."""
148+ self.assertEqual(None, self.parent)
149+ self.assertEqual(self.title, question_title)
150+ self.assertEqual(self.text, question_text)
151+ self.assertEqual(self.yes_button, gui.QtGui.QMessageBox.Yes)
152+ self.assertEqual(self.no_button, gui.QtGui.QMessageBox.No)
153+
154+ def _assert_true_question(self, question_fn, question_title,
155+ question_text):
156+ """Assert a positive answer to a question."""
157 self.answer = gui.QtGui.QMessageBox.Yes
158- self.assertTrue(gui.user_wants_to_update(), 'User wants to update.')
159- # lets assert the parameters
160- self.assertTrue(None, self.parent)
161- self.assertTrue(self.title, gui.UPDATE_TITLE)
162- self.assertTrue(self.text, gui.UPDATE_SOFTWARE)
163- self.assertTrue(self.yes_button, gui.QtGui.QMessageBox.Yes)
164- self.assertTrue(self.no_button, gui.QtGui.QMessageBox.No)
165+ self.assertTrue(question_fn())
166+ self._assert_question(question_title, question_text)
167
168- def user_wants_to_update_false(self):
169- """Test the question."""
170+ def _assert_false_question(self, question_fn, question_title,
171+ question_text):
172+ """Assert a negative answer to a question."""
173 self.answer = gui.QtGui.QMessageBox.No
174- msg = 'user_wants_to_update must return False when the answer is No.'
175- self.assertFalse(gui.user_wants_to_update(), msg)
176- # lets assert the parameters
177- self.assertTrue(None, self.parent)
178- self.assertTrue(self.title, gui.UPDATE_TITLE)
179- self.assertTrue(self.text, gui.UPDATE_SOFTWARE)
180- self.assertTrue(self.yes_button, gui.QtGui.QMessageBox.Yes)
181- self.assertTrue(self.no_button, gui.QtGui.QMessageBox.No)
182+ self.assertFalse(question_fn())
183+ self._assert_question(question_title, question_text)
184+
185+ def test_user_wants_to_update_true(self):
186+ """Test the question."""
187+ self._assert_true_question(gui.user_wants_to_update, gui.UPDATE_TITLE,
188+ gui.UPDATE_SOFTWARE)
189+
190+ def test_user_wants_to_update_false(self):
191+ """Test the question."""
192+ self._assert_false_question(gui.user_wants_to_update, gui.UPDATE_TITLE,
193+ gui.UPDATE_SOFTWARE)
194+
195+ def test_user_wants_to_remove_true(self):
196+ """Test the question."""
197+ self._assert_true_question(gui.user_wants_to_remove_old_beta,
198+ gui.REMOVE_OLD_BETA_TITLE,
199+ gui.REMOVE_OLD_BETA)
200+
201+ def test_user_wants_to_remove_false(self):
202+ """Test the question."""
203+ self._assert_false_question(gui.user_wants_to_remove_old_beta,
204+ gui.REMOVE_OLD_BETA_TITLE,
205+ gui.REMOVE_OLD_BETA)
206+
207+ def test_user_wants_to_migrate_true(self):
208+ """Test the question."""
209+ self._assert_true_question(gui.user_wants_to_migrate_data,
210+ gui.MIGRATE_OLD_BETA_DATA_TITLE,
211+ gui.MIGRATE_OLD_BETA)
212+
213+ def test_user_wants_to_migrate_false(self):
214+ """Test the question."""
215+ self._assert_false_question(gui.user_wants_to_migrate_data,
216+ gui.MIGRATE_OLD_BETA_DATA_TITLE,
217+ gui.MIGRATE_OLD_BETA)
218+
219+ def _assert_warn_user(self, warn_fn, title, text):
220+ """Assert waning a user."""
221+ self.assertEqual(None, warn_fn())
222+ self.assertEqual(None, self.parent)
223+ self.assertEqual(self.title, title)
224+ self.assertEqual(self.text, text)
225+ self.assertEqual(self.ok_button, gui.QtGui.QMessageBox.Ok)
226+
227+ def test_warn_user_data_migration(self):
228+ """Test warning the user."""
229+ self._assert_warn_user(gui.warn_user_data_not_migrated,
230+ gui.MIGRATE_OLD_BETA_DATA_TITLE,
231+ gui.MIGRATE_OLD_BETA_DATA_ERROR)
232+
233+ def test_warn_user_old_app_removal(self):
234+ """Test warning the user."""
235+ self._assert_warn_user(gui.warn_user_beta_not_removed,
236+ gui.REMOVE_OLD_BETA_TITLE,
237+ gui.REMOVE_OLD_BETA_ERROR)
238
239=== modified file 'ubuntuone_installer/gui/qt/utils/__init__.py'
240--- ubuntuone_installer/gui/qt/utils/__init__.py 2011-11-11 19:27:39 +0000
241+++ ubuntuone_installer/gui/qt/utils/__init__.py 2011-12-01 14:58:32 +0000
242@@ -42,6 +42,7 @@
243 uninstall_old_beta = windows.uninstall_old_beta
244 migrate_old_data = windows.migrate_old_data
245 check_updates = windows.check_updates
246+ perform_old_beta_data_migration = windows.perform_old_beta_data_migration
247 else:
248 from ubuntuone_installer.gui.qt.utils import linux
249 uninstall_application = linux.uninstall_application
250@@ -54,6 +55,7 @@
251 uninstall_old_beta = lambda *args, **kwargs: None
252 migrate_old_data = lambda *args, **kwargs: None
253 check_updates = lambda *args, **kwargs: None
254+ perform_old_beta_data_migration = lambda *args, **kwargs: None
255 # pylint: enable=C0103
256
257
258@@ -76,6 +78,10 @@
259 logger.info('Updates checked, stopping.')
260 stop_cb()
261 else: # No credentials
262+ # if there are no credentials it usually means that we
263+ # are installing the application, before we do anything
264+ # lets check if the old beta is present
265+ yield perform_old_beta_data_migration(gui)
266 logger.info('Got empty credentials, starting wizard app.')
267 window = gui.MainWindow(close_callback=stop_cb,
268 installing=installing)
269
270=== modified file 'ubuntuone_installer/gui/qt/utils/tests/test_common.py'
271--- ubuntuone_installer/gui/qt/utils/tests/test_common.py 2011-11-11 19:28:10 +0000
272+++ ubuntuone_installer/gui/qt/utils/tests/test_common.py 2011-12-01 14:58:32 +0000
273@@ -69,11 +69,19 @@
274
275 self.start_cp = lambda with_icon: \
276 self.called.extend(('start_cp', with_icon))
277+
278 self.patch(utils, 'start_control_panel', self.start_cp)
279
280 self.check_updates = lambda _: self.called.append('check_updates')
281+
282 self.patch(utils, 'check_updates', self.check_updates)
283
284+ self.migrate_data_cb = lambda gui: \
285+ self.called.extend(('migrate_data_cb', gui))
286+
287+ self.patch(utils, 'perform_old_beta_data_migration',
288+ self.migrate_data_cb)
289+
290 self.patch(gui, 'MainWindow', FakeWindow)
291
292 @defer.inlineCallbacks
293@@ -110,3 +118,4 @@
294 self.assertTrue(FakeWindow.visible)
295 msgs = ('Got empty credentials', 'starting wizard')
296 self.assertTrue(self.memento.check_info(*msgs))
297+ self.assertEqual(self.called, ['migrate_data_cb', gui])
298
299=== modified file 'ubuntuone_installer/gui/qt/utils/tests/test_windows.py'
300--- ubuntuone_installer/gui/qt/utils/tests/test_windows.py 2011-11-16 15:42:44 +0000
301+++ ubuntuone_installer/gui/qt/utils/tests/test_windows.py 2011-12-01 14:58:32 +0000
302@@ -37,6 +37,9 @@
303 from ubuntuone_installer.gui.qt import utils
304 from ubuntuone_installer.gui.qt.tests import BaseTestCase
305
306+# we want to test private methods
307+# pylint:disable=W0212
308+
309
310 class UninstallerTestCase(BaseTestCase):
311
312@@ -395,7 +398,7 @@
313 self.patch(utils.windows, 'deferToThread',
314 fake_msi_install_product)
315 self.assertFailure(utils.uninstall_old_beta(),
316- utils.windows.MsiException)
317+ utils.windows.UninstallException)
318 self.assertEqual(self.called['property_uid'],
319 utils.windows.OLD_BETA_UID_KEY)
320 self.assertEqual(self.called['property_name'], u'LocalPackage')
321@@ -416,7 +419,7 @@
322 self.patch(utils.windows, 'get_property_for_product',
323 fake_get_property_for_product)
324 self.assertFailure(utils.uninstall_old_beta(),
325- utils.windows.MsiException)
326+ utils.windows.UninstallException)
327 self.assertEqual(self.called['uid'], utils.windows.OLD_BETA_UID_KEY)
328 self.assertEqual(self.called['property_name'], u'LocalPackage')
329
330
331=== modified file 'ubuntuone_installer/gui/qt/utils/windows.py'
332--- ubuntuone_installer/gui/qt/utils/windows.py 2011-11-16 15:42:44 +0000
333+++ ubuntuone_installer/gui/qt/utils/windows.py 2011-12-01 14:58:32 +0000
334@@ -69,7 +69,8 @@
335 LONG_PATH_PREFIX = u'\\\\?\\'
336 AUTO_UPDATE_EXE = 'autoupdate-windows.exe'
337
338-class MsiException(Exception):
339+
340+class UninstallException(Exception):
341 """Raised when there are msi issues."""
342
343
344@@ -261,10 +262,10 @@
345 result = yield deferToThread(ctypes.windll.msi.MsiInstallProductW,
346 uninstall_path, command_line)
347 if result != ERROR_SUCCESS:
348- raise MsiException('Could not remove old beta.')
349+ raise UninstallException('Could not remove old beta.')
350 else:
351 # the local file is missing so we cannot un install it :(
352- raise MsiException('Could not remove old beta.')
353+ raise UninstallException('Could not remove old beta.')
354
355
356 def _append_long_path(path):
357@@ -306,3 +307,26 @@
358 new_full_path = os.path.join(new_location, path)
359 yield deferToThread(shutil.move, old_full_path,
360 new_full_path)
361+
362+
363+@defer.inlineCallbacks
364+def perform_old_beta_data_migration(gui):
365+ """Perform the migration from the old beta."""
366+ if is_old_beta_installed() and \
367+ gui.user_wants_to_remove_old_beta():
368+ logger.debug('Old beta is installed and user wants to remove it.')
369+ try:
370+ # uninstall the msi and migrate data
371+ yield uninstall_old_beta()
372+ # ask the user if he wants to move his data
373+ if gui.user_wants_to_migrate_data():
374+ yield migrate_old_data()
375+ except UninstallException:
376+ logger.error('Could not uninstall msi: %s')
377+ gui.warn_user_beta_not_removed()
378+ except DataMigrationException:
379+ # we did uninstall the msi \o/ but failed in the data migration.
380+ # This is not a big deal, lets just tell the user that he has to
381+ # manually do it
382+ logger.error('Could not migrate data: %s')
383+ gui.warn_user_data_not_migrated()

Subscribers

People subscribed via source and target branches