Merge lp:~suutari-olli/openlp/fix-advanced-bible-search-clear-button-giving-focus-to-quick into lp:openlp

[Azaziah]

Fixed the issue where web bible's result in traceback in "Search while typing"
(by removing one miss-placed line)

This branch also fixes the issue where the new Clear Bible search results and
Lock button give focus to Text search if it is used in "Select" tab.

--------------------------------
lp:~suutari-olli/openlp/fix-advanced-bible-search-clear-button-giving-focus-to-quick (revision 2702)
[SUCCESS] https://ci.openlp.io/job/Branch-01-Pull/1771/
[SUCCESS] https://ci.openlp.io/job/Branch-02-Functional-Tests/1682/
[SUCCESS] https://ci.openlp.io/job/Branch-03-Interface-Tests/1620/
[SUCCESS] https://ci.openlp.io/job/Branch-04a-Windows_Functional_Tests/1376/
[SUCCESS] https://ci.openlp.io/job/Branch-04b-Windows_Interface_Tests/966/
[SUCCESS] https://ci.openlp.io/job/Branch-05a-Code_Analysis/1034/
[SUCCESS] https://ci.openlp.io/job/Branch-05b-Test_Coverage/902/
[SUCCESS] https://ci.openlp.io/job/Branch-05c-Code_Analysis2/65/

[Tim Bentley (trb143)]

Looks OK but needs tests!

review: Needs Fixing

[Raoul Snyman (raoul-snyman)]

Please see my comments.

review: Needs Fixing

[Tim Bentley (trb143)]

Tests? The rest looks good though

review: Needs Fixing

[Raoul Snyman (raoul-snyman)]

Please read the unittest.mock documentation, it has lots of examples on how to use it and the best way to do it.

https://docs.python.org/3/library/unittest.mock.html

Check my comments below for the best way to use the library.

review: Needs Fixing

[Azaziah (suutari-olli)]

> Please read the unittest.mock documentation, it has lots of examples on how to
> use it and the best way to do it.
>
> https://docs.python.org/3/library/unittest.mock.html
>
> Check my comments below for the best way to use the library.

We tried to change this test to use @patch with phill, but failed.

[Phill (phill-ridout)]

> > Please read the unittest.mock documentation, it has lots of examples on how
> to
> > use it and the best way to do it.
> >
> > https://docs.python.org/3/library/unittest.mock.html
> >
> > Check my comments below for the best way to use the library.
>
> We tried to change this test to use @patch with phill, but failed.

I spent a couple hours on this and couldn't figure it out.

media_item.list_view is set in a super class
media_item.quick_search_edit is set in a method called from _setup which is patched

All from memory that is.

[Tim Bentley (trb143)]

review: Approve

[Phill (phill-ridout)]

on_lock_button_toggled is such a small method, any changes of tests for that?

Also can you include the parameters in the doc string please. (See inline comments)

review: Needs Fixing

[lp:~suutari-olli/openlp/fix-advanced-bible-search-clear-button-giving-focus-to-quick]

2703. By Azaziah on 2016-09-15

- Added param: to Lock toggle button

2704. By Azaziah on 2016-09-16

- Merged trunk on 16.9.16

2705. By Azaziah on 2016-09-16

- Added a test for toggle lock button
- Removed give focus to Select bookname on Select tab, may be confusing.

2706. By Azaziah on 2016-09-17

- Added two new tests for lock button toggle.

2707. By Azaziah on 2016-09-17

- The checking for not_called_once does not work apparently, fails in jenkins but passes in nosetests.

Unmerged revisions

=== modified file 'openlp/core/ui/exceptiondialog.py'
--- openlp/core/ui/exceptiondialog.py	2016-08-03 21:19:14 +0000
+++ openlp/core/ui/exceptiondialog.py	2016-09-16 21:44:25 +0000
@@ -97,9 +97,9 @@
             translate('OpenLP.ExceptionDialog', '<strong>Please describe what you were trying to do.</strong> '
                                                 '&nbsp;If possible, write in English.'))
         exception_part1 = (translate('OpenLP.ExceptionDialog',
-                                     '<strong>Oops, OpenLP hit a problem and couldn\'t recover!</strong> <br><br>'
-                                     '<strong>You can help </strong> the OpenLP developers to <strong>fix this</strong>'
-                                     ' by<br> sending them a <strong>bug report</strong> to {email}{newlines}'
+                                     '<strong>Oops, OpenLP hit a problem and couldn\'t recover!<br><br>'
+                                     'You can help </strong> the OpenLP developers to <strong>fix this</strong>'
+                                     ' by<br> sending them a <strong>bug report to {email}</strong>{newlines}'
                                      ).format(email='<a href = "mailto:[e-mail]" > [e-mail]</a>',
                                               newlines='<br><br>'))
         self.message_label.setText(

=== modified file 'openlp/plugins/bibles/lib/manager.py'
--- openlp/plugins/bibles/lib/manager.py	2016-09-04 16:15:57 +0000
+++ openlp/plugins/bibles/lib/manager.py	2016-09-16 21:44:25 +0000
@@ -367,7 +367,6 @@
             second_web_bible = self.db_cache[second_bible].get_object(BibleMeta, 'download_source')
         if web_bible or second_web_bible:
             # If either Bible is Web, cursor is reset to normal and search ends w/o any message.
-            self.check_search_result()
             self.application.set_normal_cursor()
             return None
         # Fetch the results from db. If no results are found, return None, no message is given for this.

=== modified file 'openlp/plugins/bibles/lib/mediaitem.py'
--- openlp/plugins/bibles/lib/mediaitem.py	2016-08-10 18:31:33 +0000
+++ openlp/plugins/bibles/lib/mediaitem.py	2016-09-16 21:44:25 +0000
@@ -254,8 +254,8 @@
         self.quickStyleComboBox.activated.connect(self.on_quick_style_combo_box_changed)
         self.advancedStyleComboBox.activated.connect(self.on_advanced_style_combo_box_changed)
         # Buttons
-        self.advancedClearButton.clicked.connect(self.on_clear_button)
-        self.quickClearButton.clicked.connect(self.on_clear_button)
+        self.advancedClearButton.clicked.connect(self.on_advanced_clear_button_clicked)
+        self.quickClearButton.clicked.connect(self.on_clear_button_clicked)
         self.advancedSearchButton.clicked.connect(self.on_advanced_search_button)
         self.quickSearchButton.clicked.connect(self.on_quick_search_button)
         # Other stuff
@@ -548,19 +548,31 @@
             self.advancedTab.setVisible(True)
             self.advanced_book_combo_box.setFocus()
 
-    def on_clear_button(self):
+    def on_clear_button_clicked(self):
         # Clear the list, then set the "No search Results" message, then clear the text field and give it focus.
         self.list_view.clear()
         self.check_search_result()
         self.quick_search_edit.clear()
         self.quick_search_edit.setFocus()
 
+    def on_advanced_clear_button_clicked(self):
+        # The same as the on_clear_button_clicked, but gives focus to Book name field in "Select" (advanced).
+        self.list_view.clear()
+        self.check_search_result()
+        self.advanced_book_combo_box.setFocus()
+
     def on_lock_button_toggled(self, checked):
-        self.quick_search_edit.setFocus()
+        """
+        Toggle the lock button, if Search tab is used, set focus to search field.
+        :param checked: The state of the toggle button. bool
+        :return: None
+        """
         if checked:
             self.sender().setIcon(self.lock_icon)
         else:
             self.sender().setIcon(self.unlock_icon)
+        if self.quickTab.isVisible():
+            self.quick_search_edit.setFocus()
 
     def on_quick_style_combo_box_changed(self):
         self.settings.layout_style = self.quickStyleComboBox.currentIndex()

=== modified file 'tests/functional/openlp_plugins/bibles/test_mediaitem.py'
--- tests/functional/openlp_plugins/bibles/test_mediaitem.py	2016-06-14 21:55:37 +0000
+++ tests/functional/openlp_plugins/bibles/test_mediaitem.py	2016-09-16 21:44:25 +0000
@@ -114,7 +114,7 @@
                 self.assertEqual(self.media_item.search_results, {})
                 self.assertEqual(self.media_item.second_search_results, {})
 
-    def on_quick_search_button_general_test(self):
+    def test_on_quick_search_button_general(self):
         """
         Test that general things, which should be called on all Quick searches are called.
         """
@@ -150,3 +150,36 @@
         self.assertEqual(2, self.media_item.quickSearchButton.setEnabled.call_count, 'Disable and Enable the button')
         self.assertEqual(1, self.media_item.check_search_result.call_count, 'Check results Should had been called once')
         self.assertEqual(1, self.app.set_normal_cursor.call_count, 'Normal cursor should had been called once')
+
+    def test_on_clear_button_clicked(self):
+        """
+        Test that the on_clear_button_clicked works properly. (Used by Bible search tab)
+        """
+        # GIVEN: Mocked list_view, check_search_results & quick_search_edit.
+        self.media_item.list_view = MagicMock()
+        self.media_item.check_search_result = MagicMock()
+        self.media_item.quick_search_edit = MagicMock()
+
+        # WHEN: on_clear_button_clicked is called
+        self.media_item.on_clear_button_clicked()
+
+        # THEN: Search result should be reset and search field should receive focus.
+        self.media_item.list_view.clear.assert_called_once_with(),
+        self.media_item.check_search_result.assert_called_once_with(),
+        self.media_item.quick_search_edit.clear.assert_called_once_with(),
+        self.media_item.quick_search_edit.setFocus.assert_called_once_with()
+
+    def test_on_lock_button_toggled_search_tab(self):
+        """
+        Test that "on_lock_button_toggled" gives focus to the right field.
+        """
+        # GIVEN: Mocked functions
+        self.media_item.sender = MagicMock()
+        self.media_item.quickTab = MagicMock()
+        self.media_item.quick_search_edit = MagicMock()
+
+        # WHEN: on_lock_button_toggled is called and quickTab.isVisible() returns = True.
+        self.media_item.on_lock_button_toggled(True)
+
+        # THEN: on_quick_search_edit should receive focus.
+        self.media_item.quick_search_edit.setFocus.assert_called_once_with()