Merge lp:~dobey/ubuntuone-control-panel/update-4-2 into lp:ubuntuone-control-panel/stable-4-2

[dobey]

[Mike McCracken]

    - Work around Qt issue where search files popup frame was not hidden after switching to another tab. (LP: #1152388)
    - Use Qt timers to delay and coalesce IPC for files search. (LP: #1150316)

[Roberto Alsina (ralsina)]

review: Approve

=== modified file 'ubuntuone/controlpanel/gui/qt/controlpanel.py'
--- ubuntuone/controlpanel/gui/qt/controlpanel.py	2012-10-18 21:45:05 +0000
+++ ubuntuone/controlpanel/gui/qt/controlpanel.py	2013-03-26 20:55:24 +0000
@@ -79,6 +79,11 @@
         self.ui.tab_widget.setTabText(
             self.ui.tab_widget.indexOf(self.ui.share_links_tab),
                 MAIN_SHARE_LINKS_TAB)
+
+        # Workaround for bug LP: 1152388
+        handler = self.ui.share_links_tab.handle_current_tab_changed
+        self.ui.tab_widget.currentChanged.connect(handler)
+
         self.ui.tab_widget.setTabText(
             self.ui.tab_widget.indexOf(self.ui.devices_tab), MAIN_DEVICES_TAB)
         self.ui.tab_widget.setTabText(
 
=== modified file 'ubuntuone/controlpanel/gui/qt/share_links.py'
--- ubuntuone/controlpanel/gui/qt/share_links.py	2012-11-02 17:15:15 +0000
+++ ubuntuone/controlpanel/gui/qt/share_links.py	2013-03-26 20:55:24 +0000
@@ -1,6 +1,6 @@
 # -*- coding: utf-8 *-*
 
-# Copyright 2012 Canonical Ltd.
+# Copyright 2012-2013 Canonical Ltd.
 #
 # This program is free software: you can redistribute it and/or modify it
 # under the terms of the GNU General Public License version 3, as published
@@ -107,6 +107,11 @@
         self.get_public_files()
         self._enhanced_line.btn_operation.hide()
 
+    def handle_current_tab_changed(self, index):
+        """Workaround for bug LP: 1152388"""
+        self.ui.line_search.clearFocus()
+        self.ui.line_search.popup.hide()
+
     @inlineCallbacks
     def share_file(self, file_path):
         """Clean the previous file share details and publish file_path."""
 
=== modified file 'ubuntuone/controlpanel/gui/qt/share_links_search.py'
--- ubuntuone/controlpanel/gui/qt/share_links_search.py	2013-01-24 21:35:59 +0000
+++ ubuntuone/controlpanel/gui/qt/share_links_search.py	2013-03-26 20:55:24 +0000
@@ -36,6 +36,7 @@
 HOME_DIR = ''
 AVOID_SECOND_ITEM = 2
 NORMAL_INCREMENT = 1
+SEARCH_TYPING_DELAY = 100       # msec
 
 
 def get_system_icon_for_filename(file_path):
@@ -63,7 +64,6 @@
         self.current_results = []
         self.page_index = 0
         self.page_size = 20
-        self.pending_call = None
         self._post_key_event = {
             QtCore.Qt.Key_Escape: lambda *args: self.popup.hide(),
             QtCore.Qt.Key_Down: self._key_down_pressed,
@@ -76,6 +76,10 @@
         self.popup.list_widget.verticalScrollBar().valueChanged.connect(
             self._scroll_fetch_more)
         self.textChanged.connect(self.handle_text_changed)
+        self._do_search_timer = QtCore.QTimer()
+        self._do_search_timer.timeout.connect(self._do_search)
+        self._do_search_timer.setInterval(SEARCH_TYPING_DELAY)
+        self._do_search_timer.setSingleShot(True)
 
         self._get_home_path()
 
@@ -94,30 +98,19 @@
     def handle_text_changed(self, text):
         """Use delayed IPC to search for filenames after user stops typing."""
 
-        # Import here to avoid getting the wrong reactor due to import
-        # order. Save in class var to ease patching for tests:
-        if not self.qtreactor:
-            self.qtreactor = __import__('twisted').internet.reactor
         text = unicode(text)
         self.page_index = 0
 
         if text == '':
             self.popup.hide()
-            if self.pending_call and self.pending_call.active():
-                self.pending_call.cancel()
-            return
-
-        if self.pending_call and self.pending_call.active():
-            self.pending_call.reset(0.1)
-            return
-
-        self.pending_call = self.qtreactor.callLater(0.1, self._do_search)
+            self._do_search_timer.stop()
+            return
+
+        self._do_search_timer.start(SEARCH_TYPING_DELAY)
 
     @inlineCallbacks
     def _do_search(self):
 
-        self.pending_call = None
-
         search_text = unicode(self.text())
         results = yield self.backend.search_files(search_text)
 
 
=== modified file 'ubuntuone/controlpanel/gui/qt/tests/test_controlpanel.py'
--- ubuntuone/controlpanel/gui/qt/tests/test_controlpanel.py	2012-10-17 07:21:28 +0000
+++ ubuntuone/controlpanel/gui/qt/tests/test_controlpanel.py	2013-03-26 20:55:24 +0000
@@ -1,6 +1,6 @@
 # -*- coding: utf-8 -*-
 #
-# Copyright 2011-2012 Canonical Ltd.
+# Copyright 2011-2013 Canonical Ltd.
 #
 # This program is free software: you can redistribute it and/or modify it
 # under the terms of the GNU General Public License version 3, as published
@@ -19,10 +19,17 @@
 from __future__ import division
 
 from twisted.internet import defer
+from PyQt4 import QtCore
+
+from ubuntuone.controlpanel import backend, cache
+from ubuntuone.controlpanel.tests import TestCase
+from mock import call, Mock
+
+from ubuntuone.controlpanel.gui.qt import share_links
 
 from ubuntuone.controlpanel.gui.qt import controlpanel as gui
 from ubuntuone.controlpanel.gui.qt.tests import (
-    SAMPLE_ACCOUNT_INFO, SAMPLE_NAME,
+    FakedControlPanelBackend, SAMPLE_ACCOUNT_INFO, SAMPLE_NAME,
 )
 from ubuntuone.controlpanel.gui.qt.tests.test_ubuntuonebin import (
     UbuntuOneBinTestCase,
@@ -204,6 +211,39 @@
             self.ui.ui.wizard.pages[self.ui.ui.wizard.license_page])
 
 
+class ControlPanelConnectionTestCase(TestCase):
+    """Test qt signal connections from controlpanel."""
+
+    @defer.inlineCallbacks
+    def setUp(self):
+        cache.Cache._shared_objects = {}
+        yield super(ControlPanelConnectionTestCase, self).setUp()
+        self.patch(backend, 'ControlBackend', FakedControlPanelBackend)
+
+        self.mock_handler = Mock(name='handle_current_tab_changed')
+        self.patch(share_links.ShareLinksPanel, 'handle_current_tab_changed',
+                   self.mock_handler)
+
+        self.ui = gui.ControlPanel()
+        self.ui.show()
+        self.addCleanup(self.ui.hide)
+        #self.addCleanup(self.ui.deleteLater)
+        self.addCleanup(QtCore.QCoreApplication.instance().processEvents)
+
+        if getattr(self.ui, 'backend', None) is not None:
+            self.addCleanup(self.ui.backend._called.clear)
+
+    def test_popup_hides_when_switching_tab(self):
+        """Test that the share_links_tab gets the signal for changed tabs"""
+        folders_index = self.ui.ui.tab_widget.indexOf(self.ui.ui.folders_tab)
+        share_index = self.ui.ui.tab_widget.indexOf(self.ui.ui.share_links_tab)
+        self.ui.ui.tab_widget.setCurrentIndex(share_index)
+        self.ui.ui.tab_widget.setCurrentIndex(folders_index)
+
+        self.assertEqual(self.mock_handler.mock_calls,
+                         [call(share_index), call(folders_index)])
+
+
 class ExternalLinkButtonsTestCase(ControlPanelTestCase):
     """The link in the go-to-web buttons are correct."""
 
 
=== modified file 'ubuntuone/controlpanel/gui/qt/tests/test_share_links.py'
--- ubuntuone/controlpanel/gui/qt/tests/test_share_links.py	2012-11-02 17:15:15 +0000
+++ ubuntuone/controlpanel/gui/qt/tests/test_share_links.py	2013-03-26 20:55:24 +0000
@@ -254,6 +254,14 @@
             os.path.basename(file_path))
         self.assertEqual(widget.ui.lbl_path.text(), file_path)
 
+    def test_hide_popup_on_tab_changed(self):
+        """Test that the popup is hidden by the tab changed signal."""
+
+        self.ui.ui.line_search.popup.show()
+        self.ui.handle_current_tab_changed(0)
+        self.assertFalse(self.ui.ui.line_search.popup.isVisible())
+        self.assertFalse(self.ui.ui.line_search.hasFocus())
+
 
 class ActionsButtonsTestCase(BaseTestCase):
     """Test the Actions Buttons."""
 
=== modified file 'ubuntuone/controlpanel/gui/qt/tests/test_share_links_search.py'
--- ubuntuone/controlpanel/gui/qt/tests/test_share_links_search.py	2013-01-24 21:35:59 +0000
+++ ubuntuone/controlpanel/gui/qt/tests/test_share_links_search.py	2013-03-26 20:55:24 +0000
@@ -24,7 +24,7 @@
 from ubuntuone.controlpanel.gui.qt import share_links_search as gui
 from ubuntuone.controlpanel.gui.qt.tests import BaseTestCase
 
-from mock import call, patch
+from mock import patch
 
 # pylint: disable=W0212
 
@@ -175,7 +175,7 @@
 
 
 class SearchingTestCase(BaseTestCase):
-    """test _do_search by itself and with multiple calls to handle_text."""
+    """Set up patches used by subclasses."""
     class_ui = gui.SearchBox
 
     @defer.inlineCallbacks
@@ -208,6 +208,9 @@
         self.mock_load_items = self.load_items_patch.start()
         self.addCleanup(self.load_items_patch.stop)
 
+
+class DoSearchTestCase(SearchingTestCase):
+    """A subclass so that MultipleSearchingTestCase doesn't also call these."""
     @defer.inlineCallbacks
     def test_do_search_text_same(self):
         """If searchbox text same after search_files call, call load_items."""
@@ -231,11 +234,37 @@
         yield d
         self.assertFalse(self.mock_load_items.called)
 
+
+class MultipleSearchingTestCase(SearchingTestCase):
+    """Test multiple fast calls to handle_text."""
+
+    @defer.inlineCallbacks
+    def setUp(self):
+        # do all this before calling super, because ui._do_search is
+        # connected to a Qt signal in the __init__ of the ui class,
+        # and we need to connect our patched version:
+        self.do_search_ended = defer.DeferredQueue()
+        # save unbound function because self.ui won't exist yet:
+        self.orig_do_search = gui.SearchBox._do_search
+
+        @defer.inlineCallbacks
+        def do_search_later():
+            # call unbound function with now-existing self.ui:
+            yield self.orig_do_search(self.ui)
+            self.do_search_ended.put("done")
+
+        self.do_search_patch = patch.object(gui.SearchBox, '_do_search')
+        self.do_search_mock = self.do_search_patch.start()
+        self.do_search_mock.side_effect = do_search_later
+        self.addCleanup(self.do_search_patch.stop)
+
+        yield super(MultipleSearchingTestCase, self).setUp()
+
     @defer.inlineCallbacks
     def test_multiple_searches_while_waiting(self):
         """Only call load_items once despite multiple quick text changes."""
 
-        # This test checks the case not covered in other tests, where
+        # This test checks a case not covered earlier, where
         # text changes once, then the text changes again, after
         # _do_search is called but before search_files has returned.
 
@@ -243,48 +272,37 @@
         # be called twice, but _load_items should only be called once,
         # by the last call to _do_search.
 
-        self.do_search_ended = defer.DeferredQueue()
-        self.orig_do_search = self.ui._do_search
-
-        @defer.inlineCallbacks
-        def do_search_later():
-            yield self.orig_do_search()
-            self.do_search_ended.put("done")
-
-        with patch.object(self.ui, '_do_search') as mock_do_search:
-            mock_do_search.side_effect = do_search_later
-
-            # call once with original query
-            self.mock_text.return_value = 'query'
-            self.ui.handle_text_changed('query')
-
-            # wait for the first delayed do_search call to call
-            # search_files and check its arg for good measure
-            search_files_query = yield self.search_files_started_q.get()
-            self.assertEqual('query', search_files_query)
-
-            # first call to search_files is paused waiting for
-            # search_files_done_q, simulating a long IPC call.
-            # the delayed call to do_search is no longer active.
-
-            # while we wait for search_files, the user changes the
-            # query, scheduling a new delayed call to do_search:
-            self.mock_text.return_value = 'query2'
-            self.ui.handle_text_changed('query2')
-
-            # release both calls to search_files:
-            self.search_files_done_q.put(['result1'])
-            self.search_files_done_q.put(['result2'])
-
-            # wait for first delayed call to do_search to finish:
-            yield self.do_search_ended.get()
-
-            # check that the second call to search_files got the right
-            # text:
-            search_files_query = yield self.search_files_started_q.get()
-            self.assertEqual('query2', search_files_query)
-            # wait for second do_search to end
-            yield self.do_search_ended.get()
+        # call once with original query
+        self.mock_text.return_value = 'query'
+        self.ui.handle_text_changed('query')
+
+        # wait for the first delayed do_search call to call
+        # search_files and check its arg for good measure
+        search_files_query = yield self.search_files_started_q.get()
+        self.assertEqual('query', search_files_query)
+
+        # first call to search_files is paused waiting for
+        # search_files_done_q, simulating a long IPC call.
+        # the delayed call to do_search is no longer active.
+
+        # while we wait for search_files, the user changes the
+        # query, scheduling a new delayed call to do_search:
+        self.mock_text.return_value = 'query2'
+        self.ui.handle_text_changed('query2')
+
+        # release both calls to search_files:
+        self.search_files_done_q.put(['result1'])
+        self.search_files_done_q.put(['result2'])
+
+        # wait for first delayed call to do_search to finish:
+        yield self.do_search_ended.get()
+
+        # check that the second call to search_files got the right
+        # text:
+        search_files_query = yield self.search_files_started_q.get()
+        self.assertEqual('query2', search_files_query)
+        # wait for second do_search to end
+        yield self.do_search_ended.get()
 
         # check that _load_items is only called once, and that it's
         # using only the results from the last call to search_files:
@@ -292,83 +310,6 @@
         self.assertEqual(['result2'], self.ui.current_results)
 
 
-class TextChangedTestCase(BaseTestCase):
-    """Test handle_text_changed scheduling pending calls."""
-
-    class_ui = gui.SearchBox
-
-    @defer.inlineCallbacks
-    def setUp(self):
-        yield super(TextChangedTestCase, self).setUp()
-
-        # patch this way instead of using decorators because we need
-        # to patch self.ui.popup, which isn't defined yet when the
-        # decorators run.
-        self.qtreactor_patch = patch.object(gui.SearchBox, 'qtreactor')
-        self.mock_qtreactor = self.qtreactor_patch.start()
-        self.addCleanup(self.qtreactor_patch.stop)
-
-        self.popup_patch = patch.object(self.ui, 'popup')
-        self.mock_popup = self.popup_patch.start()
-        self.addCleanup(self.popup_patch.stop)
-
-    def test_empty_text_no_pending(self):
-        """arg='' with no pending call only hides the popup."""
-        self.ui.handle_text_changed('')
-        self.mock_popup.hide.assert_called_once_with()
-        self.assertFalse(self.mock_qtreactor.callLater.called)
-
-    def test_empty_text_with_pending(self):
-        """arg='' with a pending call hides the popup and cancels pending."""
-        with patch.object(self.ui, 'pending_call') as mock_pending_call:
-            mock_pending_call.active.return_value = True
-            self.ui.handle_text_changed('')
-            mock_pending_call.active.assert_called_once_with()
-            mock_pending_call.cancel.assert_called_once_with()
-        self.mock_popup.hide.assert_called_once_with()
-        self.assertFalse(self.mock_qtreactor.callLater.called)
-
-    def test_empty_text_with_inactive_pending(self):
-        """arg='' with a pending call hides the popup and cancels pending."""
-        with patch.object(self.ui, 'pending_call') as mock_pending_call:
-            mock_pending_call.active.return_value = False
-            self.ui.handle_text_changed('')
-            mock_pending_call.active.assert_called_once_with()
-            self.assertFalse(mock_pending_call.cancel.called)
-        self.mock_popup.hide.assert_called_once_with()
-        self.assertFalse(self.mock_qtreactor.callLater.called)
-
-    def test_nonempty_text_with_no_pending(self):
-        """arg='b' with no pending call schedules a call."""
-        self.ui.handle_text_changed('b')
-        self.assertEqual([call(0.1, self.ui._do_search)],
-                         self.mock_qtreactor.callLater.mock_calls)
-
-    def test_nonempty_text_with_inactive_pending(self):
-        """call an inactive (called already) call schedules another call."""
-        with patch.object(self.ui, 'pending_call') as mock_pending_call:
-            mock_pending_call.active.return_value = False
-            self.ui.handle_text_changed('b')
-            mock_pending_call.active.assert_called_once_with()
-            self.assertFalse(mock_pending_call.reset.called)
-
-        self.assertFalse(self.mock_popup.hide.called)
-
-        self.assertEqual([call(0.1, self.ui._do_search)],
-                         self.mock_qtreactor.callLater.mock_calls)
-
-    def test_nonempty_text_with_pending(self):
-        """arg='' with a pending call hides the popup and resets pending."""
-        with patch.object(self.ui, 'pending_call') as mock_pending_call:
-            mock_pending_call.active.return_value = True
-            self.ui.handle_text_changed('b')
-            mock_pending_call.active.assert_called_once_with()
-            mock_pending_call.reset.assert_called_once_with(0.1)
-
-        self.assertFalse(self.mock_popup.hide.called)
-        self.assertFalse(self.mock_qtreactor.callLater.called)
-
-
 class FileItemTestCase(BaseTestCase):
     """Test the File Item."""