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

Proposed by Azaziah
Status: Superseded
Proposed branch: lp:~suutari-olli/openlp/fix-advanced-bible-search-clear-button-giving-focus-to-quick
Merge into: lp:openlp
Diff against target: 101 lines (+39/-8)
3 files modified
openlp/core/ui/exceptiondialog.py (+3/-3)
openlp/plugins/bibles/lib/mediaitem.py (+17/-4)
tests/functional/openlp_plugins/bibles/test_mediaitem.py (+19/-1)
To merge this branch: bzr merge lp:~suutari-olli/openlp/fix-advanced-bible-search-clear-button-giving-focus-to-quick
Reviewer Review Type Date Requested Status
Raoul Snyman Needs Fixing
Tim Bentley Pending
Review via email: mp+303487@code.launchpad.net

This proposal supersedes a proposal from 2016-08-19.

This proposal has been superseded by a proposal from 2016-09-09.

To post a comment you must log in.
Revision history for this message
Tim Bentley (trb143) wrote : Posted in a previous version of this proposal

Looks OK but needs tests!

review: Needs Fixing
Revision history for this message
Raoul Snyman (raoul-snyman) wrote : Posted in a previous version of this proposal

Please see my comments.

review: Needs Fixing
Revision history for this message
Tim Bentley (trb143) wrote : Posted in a previous version of this proposal

Tests? The rest looks good though

review: Needs Fixing
Revision history for this message
Raoul Snyman (raoul-snyman) wrote :

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
Revision history for this message
Azaziah (suutari-olli) wrote :

> 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.

Revision history for this message
Phill (phill-ridout) wrote :

> > 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.

2699. By Azaziah

- Fixed the test

2700. By Azaziah

- Combined some <strong> tags in expection dialogue.
- Added new line to the end of the test.

2701. By Azaziah

- Merged trunk on 9.9.16

2702. By Azaziah

- Fixed a bug where web bible's trigger traceback in search while typing.

2703. By Azaziah

- Added param: to Lock toggle button

2704. By Azaziah

- Merged trunk on 16.9.16

2705. By Azaziah

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

2706. By Azaziah

- Added two new tests for lock button toggle.

2707. By Azaziah

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

Unmerged revisions

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'openlp/core/ui/exceptiondialog.py'
2--- openlp/core/ui/exceptiondialog.py 2016-08-03 21:19:14 +0000
3+++ openlp/core/ui/exceptiondialog.py 2016-09-09 07:58:57 +0000
4@@ -97,9 +97,9 @@
5 translate('OpenLP.ExceptionDialog', '<strong>Please describe what you were trying to do.</strong> '
6 '&nbsp;If possible, write in English.'))
7 exception_part1 = (translate('OpenLP.ExceptionDialog',
8- '<strong>Oops, OpenLP hit a problem and couldn\'t recover!</strong> <br><br>'
9- '<strong>You can help </strong> the OpenLP developers to <strong>fix this</strong>'
10- ' by<br> sending them a <strong>bug report</strong> to {email}{newlines}'
11+ '<strong>Oops, OpenLP hit a problem and couldn\'t recover!<br><br>'
12+ 'You can help </strong> the OpenLP developers to <strong>fix this</strong>'
13+ ' by<br> sending them a <strong>bug report to {email}</strong>{newlines}'
14 ).format(email='<a href = "mailto:bugs@openlp.org" > bugs@openlp.org</a>',
15 newlines='<br><br>'))
16 self.message_label.setText(
17
18=== modified file 'openlp/plugins/bibles/lib/mediaitem.py'
19--- openlp/plugins/bibles/lib/mediaitem.py 2016-08-10 18:31:33 +0000
20+++ openlp/plugins/bibles/lib/mediaitem.py 2016-09-09 07:58:57 +0000
21@@ -254,8 +254,8 @@
22 self.quickStyleComboBox.activated.connect(self.on_quick_style_combo_box_changed)
23 self.advancedStyleComboBox.activated.connect(self.on_advanced_style_combo_box_changed)
24 # Buttons
25- self.advancedClearButton.clicked.connect(self.on_clear_button)
26- self.quickClearButton.clicked.connect(self.on_clear_button)
27+ self.advancedClearButton.clicked.connect(self.on_advanced_clear_button_clicked)
28+ self.quickClearButton.clicked.connect(self.on_clear_button_clicked)
29 self.advancedSearchButton.clicked.connect(self.on_advanced_search_button)
30 self.quickSearchButton.clicked.connect(self.on_quick_search_button)
31 # Other stuff
32@@ -548,19 +548,32 @@
33 self.advancedTab.setVisible(True)
34 self.advanced_book_combo_box.setFocus()
35
36- def on_clear_button(self):
37+ def on_clear_button_clicked(self):
38 # Clear the list, then set the "No search Results" message, then clear the text field and give it focus.
39 self.list_view.clear()
40 self.check_search_result()
41 self.quick_search_edit.clear()
42 self.quick_search_edit.setFocus()
43
44+ def on_advanced_clear_button_clicked(self):
45+ # The same as the on_clear_button_clicked, but gives focus to Book name field in "Select" (advanced).
46+ self.list_view.clear()
47+ self.check_search_result()
48+ self.advanced_book_combo_box.setFocus()
49+
50 def on_lock_button_toggled(self, checked):
51- self.quick_search_edit.setFocus()
52+ """
53+ Toggle the lock button, if Search tab is used, set focus to search field, if Select tab is used,
54+ give focus to Bible book name field.
55+ """
56 if checked:
57 self.sender().setIcon(self.lock_icon)
58 else:
59 self.sender().setIcon(self.unlock_icon)
60+ if self.quickTab.isVisible():
61+ self.quick_search_edit.setFocus()
62+ else:
63+ self.advanced_book_combo_box.setFocus()
64
65 def on_quick_style_combo_box_changed(self):
66 self.settings.layout_style = self.quickStyleComboBox.currentIndex()
67
68=== modified file 'tests/functional/openlp_plugins/bibles/test_mediaitem.py'
69--- tests/functional/openlp_plugins/bibles/test_mediaitem.py 2016-06-14 21:55:37 +0000
70+++ tests/functional/openlp_plugins/bibles/test_mediaitem.py 2016-09-09 07:58:57 +0000
71@@ -114,7 +114,7 @@
72 self.assertEqual(self.media_item.search_results, {})
73 self.assertEqual(self.media_item.second_search_results, {})
74
75- def on_quick_search_button_general_test(self):
76+ def test_on_quick_search_button_general(self):
77 """
78 Test that general things, which should be called on all Quick searches are called.
79 """
80@@ -150,3 +150,21 @@
81 self.assertEqual(2, self.media_item.quickSearchButton.setEnabled.call_count, 'Disable and Enable the button')
82 self.assertEqual(1, self.media_item.check_search_result.call_count, 'Check results Should had been called once')
83 self.assertEqual(1, self.app.set_normal_cursor.call_count, 'Normal cursor should had been called once')
84+
85+ def test_on_clear_button_clicked(self):
86+ """
87+ Test that the on_clear_button_clicked works properly. (Used by Bible search tab)
88+ """
89+ # GIVEN: Mocked list_view, check_search_results & quick_search_edit.
90+ self.media_item.list_view = MagicMock()
91+ self.media_item.check_search_result = MagicMock()
92+ self.media_item.quick_search_edit = MagicMock()
93+
94+ # WHEN: on_clear_button_clicked is called
95+ self.media_item.on_clear_button_clicked()
96+
97+ # THEN: Search result should be reset and search field should receive focus.
98+ self.media_item.list_view.clear.assert_called_once_with(),
99+ self.media_item.check_search_result.assert_called_once_with(),
100+ self.media_item.quick_search_edit.clear.assert_called_once_with(),
101+ self.media_item.quick_search_edit.setFocus.assert_called_once_with()