Merge lp:~diegosarmentero/ubuntuone-windows-installer/close-on-license-again into lp:ubuntuone-windows-installer

Proposed by Diego Sarmentero
Status: Merged
Approved by: Roberto Alsina
Approved revision: 78
Merged at revision: 80
Proposed branch: lp:~diegosarmentero/ubuntuone-windows-installer/close-on-license-again
Merge into: lp:ubuntuone-windows-installer
Diff against target: 43 lines (+6/-6)
2 files modified
ubuntuone_installer/gui/qt/gui.py (+2/-2)
ubuntuone_installer/gui/qt/tests/test_gui.py (+4/-4)
To merge this branch: bzr merge lp:~diegosarmentero/ubuntuone-windows-installer/close-on-license-again
Reviewer Review Type Date Requested Status
Roberto Alsina (community) Approve
Manuel de la Peña (community) Approve
Review via email: mp+78596@code.launchpad.net

Commit message

Fixed: Wizard is not closed on license agreement screen (LP: #835167).

Description of the change

Fixed: Wizard is not closed on license agreement screen (LP: #835167).

To post a comment you must log in.
Revision history for this message
Manuel de la Peña (mandel) wrote :

What happened here is that the return value of the dialog changed. I think that the best way to approach this would be to have a couple of constants against the ones we can compare. For example:

if AreYouSure(self).exec_() == AreYouSure(self).YES:
    print 'You are sure!'

That way if the value returned changed the code is still valid and the reponsability of ensuring that the dialog works in on the dialog developer and not the user.

review: Needs Fixing
Revision history for this message
Manuel de la Peña (mandel) wrote :

> What happened here is that the return value of the dialog changed. I think
> that the best way to approach this would be to have a couple of constants
> against the ones we can compare. For example:
>
> if AreYouSure(self).exec_() == AreYouSure.YES:
> print 'You are sure!'
>
> That way if the value returned changed the code is still valid and the
> reponsability of ensuring that the dialog works in on the dialog developer and
> not the user.

Remove the (self), sorry for the typo, stupid copy and paste :P

78. By Diego Sarmentero

Improved AreYouSure result comparison.

Revision history for this message
Manuel de la Peña (mandel) wrote :

Much nicer!!!

review: Approve
Revision history for this message
Roberto Alsina (ralsina) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'ubuntuone_installer/gui/qt/gui.py'
--- ubuntuone_installer/gui/qt/gui.py 2011-09-29 13:26:44 +0000
+++ ubuntuone_installer/gui/qt/gui.py 2011-10-07 17:42:17 +0000
@@ -455,12 +455,12 @@
455 if result == 1: # Cancelled455 if result == 1: # Cancelled
456 qt.utils.start_control_panel()456 qt.utils.start_control_panel()
457 elif self.currentId() == self.LICENSE_PAGE:457 elif self.currentId() == self.LICENSE_PAGE:
458 if not AreYouSure(self).exec_():458 if AreYouSure(self).exec_() == QtGui.QDialog.Accepted:
459 qt.utils.uninstall_application()459 qt.utils.uninstall_application()
460 else:460 else:
461 return461 return
462 elif self.currentId() != self.CONGRATULATIONS_PAGE:462 elif self.currentId() != self.CONGRATULATIONS_PAGE:
463 if not AreYouSure(self).exec_():463 if AreYouSure(self).exec_() == QtGui.QDialog.Rejected:
464 return464 return
465 if self.close_callback is not None:465 if self.close_callback is not None:
466 self.close_callback()466 self.close_callback()
467467
=== modified file 'ubuntuone_installer/gui/qt/tests/test_gui.py'
--- ubuntuone_installer/gui/qt/tests/test_gui.py 2011-09-29 13:26:44 +0000
+++ ubuntuone_installer/gui/qt/tests/test_gui.py 2011-10-07 17:42:17 +0000
@@ -220,8 +220,8 @@
220 self.ui.restart()220 self.ui.restart()
221 self.ui.show()221 self.ui.show()
222 self.addCleanup(self.ui.hide)222 self.addCleanup(self.ui.hide)
223 gui.AreYouSure.result = 0223 gui.AreYouSure.result = 1
224 self.patch(utils, "uninstall_application", self._set_called)224 self.patch(gui.qt.utils, "uninstall_application", self._set_called)
225 self.ui.done(result=0)225 self.ui.done(result=0)
226 self.assertTrue(self._called)226 self.assertTrue(self._called)
227227
@@ -231,8 +231,8 @@
231 self.ui.restart()231 self.ui.restart()
232 self.ui.show()232 self.ui.show()
233 self.addCleanup(self.ui.hide)233 self.addCleanup(self.ui.hide)
234 gui.AreYouSure.result = 0234 gui.AreYouSure.result = 1
235 self.patch(utils, "uninstall_application", self._set_called)235 self.patch(gui.qt.utils, "uninstall_application", self._set_called)
236 self.ui.done(result=0)236 self.ui.done(result=0)
237 self.assertTrue(self._called)237 self.assertTrue(self._called)
238238

Subscribers

People subscribed via source and target branches