Merge lp:~mandel/ubuntuone-control-panel/autoupdate into lp:ubuntuone-control-panel

[Manuel de la Peña]

- Added simple autoupdate implementation based on the code found in the ubutnuone-windows-installer project (LP: #971455).

- Added simple autoupdate implementation based on the code found in the ubutnuone-windows-installer project (LP: #971455).

[lp:~mandel/ubuntuone-control-panel/autoupdate]

309. By Manuel de la Peña on 2012-04-02

Pass the gui module to check updates.

[Brian Curtin (brian.curtin)]

I think the UI that gets displayed when an update is found needs some work. Rather than "There is a new update available", we need to ask a question if they want to install it since we have yes/no buttons.

Something like "There is a new update available. Would you like to install it?"

review: Needs Fixing

[lp:~mandel/ubuntuone-control-panel/autoupdate]

310. By Manuel de la Peña on 2012-04-02

Ask a question when you do as a question.

[Brian Curtin (brian.curtin)]

That's better. The code looks fine and the IRL tests have been working.

review: Approve

[Leo Arias (elopio)]

I followed your instructions and got the update message. As Brian said, the message is better as a question and seems to be working fine.
Were we just testing that the message appears or the actual upgrade? If it was just the first, make this a +1. If it was the upgrade, I got an error saying that it couldn't copy a file. Tomorrow morning I'm available for more testing if needed, just ping me.

review: Needs Information

[Manuel de la Peña (mandel)]

The most probable cause of the error is that the pacakage found on the server side is not an update or that no package was found, lets make sure whenever you are in IRC.

[Natalia Bidart (nataliabidart)]

The module utils can not depend on the module gui.qt, since utils is being distributed in the base package which is in the Ubuntu default install. The allowed dependency is the other way around: gui.qt depends (already) on utils.
Also, this branch generates a circular dependency between utils -> gui.qt -> utils.

So, the solution to this is having the code in MainWindow should what check_updates does, importing utils and calling utils.are_updates_present() and utils.perform_update(), showing the confirmation dialog in between.

As soon as this is fixed, I'm happy to re-review.

review: Needs Fixing

[Manuel de la Peña (mandel)]

Please take another look because there is no dependency between utils and the gui modules. The utils.check_updates takes a parameter which is the gui module to use that has the ask question method, the check_updates method is used the main.windows module which has a dependency with the qt.gui module.

This means that:

* Utils does not depend on any gui module. The only dependency is found in the windows tests which can be remove by not using patch and just passing and object that implements the required gui method.
* All logic is in the utils module.
* There is a dependency between windows.main and qt.gui.

Do I make sense?

[Natalia Bidart (nataliabidart)]

> * Utils does not depend on any gui module. The only dependency is found in the
> windows tests which can be remove by not using patch and just passing and
> object that implements the required gui method.
> * All logic is in the utils module.
> * There is a dependency between windows.main and qt.gui.
>
> Do I make sense?

Yes, sorry, I stated the dependency wrong. Anyways, the ubuntu_sso.main module can not depend on the gui.qt module, since the former is part of the base package, like I mentioned before.
So, the way to solve this is having the gui.qt module doing the check and raising any needed confirmation dialog, so we keep having the gui.qt module depending on utils, like we do so far.

Since you're in holidays and we need this ASAP, I'm proposing a fix for this at:

https://code.launchpad.net/~nataliabidart/ubuntuone-control-panel/autoupdate/+merge/100989

you can review it if you like :-P (but no need to do so).

=== modified file 'ubuntuone/controlpanel/gui/qt/gui.py'
--- ubuntuone/controlpanel/gui/qt/gui.py	2012-03-29 19:05:02 +0000
+++ ubuntuone/controlpanel/gui/qt/gui.py	2012-04-02 17:43:21 +0000
@@ -31,6 +31,10 @@
 # pylint: enable=E0611
 
 U1_DOTDESKTOP = "ubuntuone-installer.desktop"
+# TODO: make strings translatable after the linux deadlines
+UPDATE_TITLE = 'Ubuntu One'
+UPDATE_SOFTWARE = ('There is a new update available.'
+                   ' Would you like to install it?')
 
 
 class MainWindow(QtGui.QMainWindow):
@@ -111,3 +115,10 @@
     else:
         icon = None
     return icon, window
+
+
+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)
+    return result == QtGui.QMessageBox.Yes

=== modified file 'ubuntuone/controlpanel/gui/qt/main/windows.py'
--- ubuntuone/controlpanel/gui/qt/main/windows.py	2012-03-15 01:25:08 +0000
+++ ubuntuone/controlpanel/gui/qt/main/windows.py	2012-04-02 17:43:21 +0000
@@ -17,6 +17,8 @@
 """Main method to be used on windows."""
 
 from PyQt4 import QtGui
+from ubuntuone.controlpanel.gui.qt import gui
+from ubuntuone.controlpanel.utils import check_updates
 
 
 def main(app):
@@ -32,6 +34,8 @@
 def main_start(app):
     """Start the mainloop."""
     from twisted.internet import reactor
+    # add the check of the updates to be executed asap
+    reactor.callWhenRunning(check_updates, gui)
     reactor.run()
 
 

=== modified file 'ubuntuone/controlpanel/gui/qt/tests/__init__.py'
--- ubuntuone/controlpanel/gui/qt/tests/__init__.py	2012-03-28 12:06:28 +0000
+++ ubuntuone/controlpanel/gui/qt/tests/__init__.py	2012-04-02 17:43:21 +0000
@@ -210,6 +210,13 @@
         cls.kwargs = kw
         return cls.response
 
+    @classmethod
+    def question(cls, *a, **kw):
+        """Simulate a question message."""
+        cls.args = a
+        cls.kwargs = kw
+        return cls.response
+
     # Invalid name "setDetailedText", "setDefaultButton"
     # pylint: disable=C0103
 

=== modified file 'ubuntuone/controlpanel/gui/qt/tests/test_gui.py'
--- ubuntuone/controlpanel/gui/qt/tests/test_gui.py	2012-03-26 13:30:07 +0000
+++ ubuntuone/controlpanel/gui/qt/tests/test_gui.py	2012-04-02 17:43:21 +0000
@@ -18,10 +18,11 @@
 
 """Tests for the Qt UI."""
 
+from PyQt4 import QtGui
 from twisted.internet import defer
 
 from ubuntuone.controlpanel.gui.qt import gui
-from ubuntuone.controlpanel.gui.qt.tests import BaseTestCase
+from ubuntuone.controlpanel.gui.qt.tests import BaseTestCase, FakedDialog
 
 
 class FakeEntry(object):
@@ -131,3 +132,21 @@
     def test_add_to_autostart(self):
         """Test that the add_to_autostart function is called when CP opens."""
         self.assertTrue(self._called)
+
+
+class UserWantsUpdateTestCase(BaseTestCase):
+    """Test the function used to ask the user if he wants to update."""
+
+    def test_user_accepts(self):
+        """Assert the result when the user accepts."""
+        FakedDialog.response = QtGui.QMessageBox.Yes
+        self.assertTrue(gui.user_wants_to_update())
+        self.assertIn(gui.UPDATE_TITLE, FakedDialog.args)
+        self.assertIn(gui.UPDATE_SOFTWARE, FakedDialog.args)
+
+    def test_user_rejects(self):
+        """Assert the result when the user rejects."""
+        FakedDialog.response = QtGui.QMessageBox.No
+        self.assertFalse(gui.user_wants_to_update())
+        self.assertIn(gui.UPDATE_TITLE, FakedDialog.args)
+        self.assertIn(gui.UPDATE_SOFTWARE, FakedDialog.args)

=== modified file 'ubuntuone/controlpanel/utils/__init__.py'
--- ubuntuone/controlpanel/utils/__init__.py	2012-03-29 17:58:28 +0000
+++ ubuntuone/controlpanel/utils/__init__.py	2012-04-02 17:43:21 +0000
@@ -39,6 +39,7 @@
     from ubuntuone.controlpanel.utils import windows
     add_to_autostart = windows.add_to_autostart
     are_updates_present = windows.are_updates_present
+    check_updates = windows.check_updates
     default_folders = windows.default_folders
     perform_update = windows.perform_update
     uninstall_application = windows.uninstall_application
@@ -46,6 +47,7 @@
     from ubuntuone.controlpanel.utils import linux
     add_to_autostart = no_op
     are_updates_present = no_op
+    check_updates = no_op
     default_folders = linux.default_folders
     perform_update = no_op
     uninstall_application = no_op

=== modified file 'ubuntuone/controlpanel/utils/tests/test_windows.py'
--- ubuntuone/controlpanel/utils/tests/test_windows.py	2012-04-02 17:41:54 +0000
+++ ubuntuone/controlpanel/utils/tests/test_windows.py	2012-04-02 17:43:21 +0000
@@ -22,6 +22,8 @@
 from twisted.internet import defer
 
 from ubuntuone.controlpanel import utils
+from ubuntuone.controlpanel.gui.qt import gui
+from ubuntuone.controlpanel.utils import windows
 from ubuntuone.controlpanel.tests import TestCase
 
 # let me use protected methods
@@ -341,3 +343,61 @@
     """Test the uninstall_application helper when sys is not frozen."""
 
     frozen = None
+
+
+class CheckUpdatesTestCase(TestCase):
+    """Test the code that is used for the auto update check process."""
+
+    @defer.inlineCallbacks
+    def setUp(self):
+        """Set the different tests."""
+        yield super(CheckUpdatesTestCase, self).setUp()
+        self.are_present = False
+        self.user_wants_update = False
+        self.called = []
+
+        def fake_are_present():
+            """Fake are updates present."""
+            self.called.append('are_updates_present')
+            return self.are_present
+
+        def fake_perform_update():
+            """Fake perform update."""
+            self.called.append('perform_update')
+
+        def fake_user_wants_to_update():
+            """Fake user_wants_to_update."""
+            self.called.append('user_wants_to_update')
+            return self.user_wants_update
+
+        self.patch(windows, 'are_updates_present', fake_are_present)
+        self.patch(windows, 'perform_update', fake_perform_update)
+        self.patch(gui, 'user_wants_to_update', fake_user_wants_to_update)
+
+    @defer.inlineCallbacks
+    def test_check_updates_not_present(self):
+        """Test the method that interacts with the user."""
+        self.are_present = False
+        yield utils.check_updates(gui)
+        self.assertTrue('are_updates_present' in self.called)
+        self.assertFalse('perform_update' in self.called)
+
+    @defer.inlineCallbacks
+    def test_check_updates_present_not_user(self):
+        """Test the method that interacts with the user."""
+        self.are_present = True
+        self.user_wants_update = False
+        yield utils.check_updates(gui)
+        self.assertTrue('are_updates_present' in self.called)
+        self.assertTrue('user_wants_to_update' in self.called)
+        self.assertFalse('perform_update' in self.called)
+
+    @defer.inlineCallbacks
+    def test_check_updates_present_user(self):
+        """Test the method that interacts with the user."""
+        self.are_present = True
+        self.user_wants_update = True
+        yield utils.check_updates(gui)
+        self.assertTrue('are_updates_present' in self.called)
+        self.assertTrue('user_wants_to_update' in self.called)
+        self.assertTrue('perform_update' in self.called)

=== modified file 'ubuntuone/controlpanel/utils/windows.py'
--- ubuntuone/controlpanel/utils/windows.py	2012-04-02 17:41:54 +0000
+++ ubuntuone/controlpanel/utils/windows.py	2012-04-02 17:43:21 +0000
@@ -87,6 +87,23 @@
     defer.returnValue(result)
 
 
[e-mail]
+def check_updates(gui):
+    """Check for updates."""
+    logger.info('Checking for updates.')
+    # the idea is simple, check if there are updates, ask user if we perform
+    # the update and continue.
+    are_present = yield are_updates_present()
+    logger.debug('Updates present: %r', are_updates_present)
+    if are_present:
+        logger.info('Updates are present.')
+        if gui.user_wants_to_update():
+            logger.info('Performing auto-update in diff process.')
+            perform_update()
+    else:
+        logger.info('No updates are present.')
+
+
 def default_folders(user_home=None):
     """Return a list of the folders to add by default."""
     # as per http://msdn.microsoft.com/en-us/library/windows/desktop/bb762181,