Merge lp:~suutari-olli/openlp/bug-fixes-2-4-3 into lp:openlp

Proposed by Azaziah on 2016-10-17
Status: Merged
Merged at revision: 2706
Proposed branch: lp:~suutari-olli/openlp/bug-fixes-2-4-3
Merge into: lp:openlp
Diff against target: 302 lines (+90/-90)
7 files modified
openlp/core/__init__.py (+40/-6)
openlp/core/lib/__init__.py (+15/-22)
openlp/core/ui/advancedtab.py (+0/-21)
openlp/core/ui/lib/treewidgetwithdnd.py (+6/-1)
openlp/core/ui/slidecontroller.py (+2/-0)
tests/functional/openlp_core_lib/test_lib.py (+25/-37)
tests/functional/openlp_plugins/bibles/test_swordimport.py (+2/-3)
To merge this branch: bzr merge lp:~suutari-olli/openlp/bug-fixes-2-4-3
Reviewer Review Type Date Requested Status
Tim Bentley Needs Fixing on 2016-10-17
Tomas Groth Needs Fixing on 2016-10-17
Raoul Snyman 2016-10-17 Approve on 2016-10-17
Review via email: mp+308596@code.launchpad.net

This proposal supersedes a proposal from 2016-10-04.

Description of the change

This branch fixes the following bugs:
Bug #1487788: Importing photos does not give focus to OpenLP
Bug #1512040: Loop tooltip gets stuck to "Stop playing..."
Bug #1513490: List of authors uses localized "and" instead of English
Bug #1624661: Missing DB in unmounted disk results in Traceback

lp:~suutari-olli/openlp/bug-fixes-2-4-3 (revision 2721)
[SUCCESS] https://ci.openlp.io/job/Branch-01-Pull/1798/
[SUCCESS] https://ci.openlp.io/job/Branch-02-Functional-Tests/1709/
[SUCCESS] https://ci.openlp.io/job/Branch-03-Interface-Tests/1647/
[SUCCESS] https://ci.openlp.io/job/Branch-04a-Windows_Functional_Tests/1403/
[SUCCESS] https://ci.openlp.io/job/Branch-04b-Windows_Interface_Tests/993/
[SUCCESS] https://ci.openlp.io/job/Branch-05a-Code_Analysis/1061/
[SUCCESS] https://ci.openlp.io/job/Branch-05b-Test_Coverage/929/
[SUCCESS] https://ci.openlp.io/job/Branch-05c-Code_Analysis2/90/

To post a comment you must log in.
Raoul Snyman (raoul-snyman) wrote :

Looks good, Olli!

review: Approve
Tomas Groth (tomasgroth) wrote :

Looks good, but just a little text fix...

review: Needs Fixing
Tim Bentley (trb143) wrote :

See below

review: Needs Fixing
Azaziah (suutari-olli) wrote :

Answered to trb143's notes.

2722. By Azaziah on 2016-10-17

- Modified the def create_separated_list by suggestions of TRB143

Raoul Snyman (raoul-snyman) wrote :

>The problem with the word "and" is that in English "," is used before
>it, where as in many other languages "," is never used before "and". To
>make it even more complicated there's no "," before and if the list
>only has two items.

Actually, it depends on the person. I personally hold to "no comma before 'and'", but my wife insists on adding the comma. Both are correct.

--
Raoul Snyman
082 550 3754
<email address hidden>
Sent from my Android device with K-9 Mail. Please excuse my brevity.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'openlp/core/__init__.py'
2--- openlp/core/__init__.py 2016-04-27 12:49:55 +0000
3+++ openlp/core/__init__.py 2016-10-17 19:43:08 +0000
4@@ -177,6 +177,40 @@
5 self.shared_memory.create(1)
6 return False
7
8+ def is_data_path_missing(self):
9+ """
10+ Check if the data folder path exists.
11+ """
12+ data_folder_path = AppLocation.get_data_path()
13+ if not os.path.exists(data_folder_path):
14+ log.critical('Database was not found in: {path}'.format(path=data_folder_path))
15+ status = QtWidgets.QMessageBox.critical(None, translate('OpenLP', 'Data Directory Error'),
16+ translate('OpenLP', 'OpenLP data folder was not found in:\n\n{path}'
17+ '\n\nThe location of the data folder was '
18+ 'previously changed from the OpenLP\'s\n'
19+ 'default location. If the data was stored on '
20+ 'removable device, that device\nneeds to be '
21+ 'made available.\n\n You may reset the data '
22+ 'location back to the default settings, '
23+ 'or you can try to make the current location '
24+ 'available.\n\n Do you want to reset the '
25+ 'default data location?\n\n Click "No" to close'
26+ 'OpenLP so you can try to fix the the problem.'
27+ '\n Click "Yes" to reset the default data '
28+ 'location and start OpenLP.')
29+ .format(path=data_folder_path),
30+ QtWidgets.QMessageBox.StandardButtons(QtWidgets.QMessageBox.Yes |
31+ QtWidgets.QMessageBox.No),
32+ QtWidgets.QMessageBox.No)
33+ if status == QtWidgets.QMessageBox.No:
34+ # If answer was "No", return "True", it will shutdown OpenLP in def main
35+ log.info('User requested termination')
36+ return True
37+ # If answer was "Yes", remove the custom data path thus resetting the default location.
38+ Settings().remove('advanced/data path')
39+ log.info('Database location has been reset to the default settings.')
40+ return False
41+
42 def hook_exception(self, exc_type, value, traceback):
43 """
44 Add an exception hook so that any uncaught exceptions are displayed in this window rather than somewhere where
45@@ -208,8 +242,8 @@
46 # If data_version is different from the current version ask if we should backup the data folder
47 elif data_version != openlp_version:
48 if QtWidgets.QMessageBox.question(None, translate('OpenLP', 'Backup'),
49- translate('OpenLP', 'OpenLP has been upgraded, do you want to create '
50- 'a backup of OpenLPs data folder?'),
51+ translate('OpenLP', 'OpenLP has been upgraded, do you want to create\n'
52+ 'a backup of the old data folder?'),
53 QtWidgets.QMessageBox.Yes | QtWidgets.QMessageBox.No,
54 QtWidgets.QMessageBox.Yes) == QtWidgets.QMessageBox.Yes:
55 # Create copy of data folder
56@@ -223,8 +257,8 @@
57 translate('OpenLP', 'Backup of the data folder failed!'))
58 return
59 message = translate('OpenLP',
60- 'A backup of the data folder has been created'
61- 'at {text}').format(text=data_folder_backup_path)
62+ 'A backup of the data folder has been created at:\n\n'
63+ '{text}').format(text=data_folder_backup_path)
64 QtWidgets.QMessageBox.information(None, translate('OpenLP', 'Backup'), message)
65
66 # Update the version in the settings
67@@ -368,8 +402,8 @@
68 Registry.create()
69 Registry().register('application', application)
70 application.setApplicationVersion(get_application_version()['version'])
71- # Instance check
72- if application.is_already_running():
73+ # If user answers "No" to already running or missing db dialogue, shutdown OpenLP.
74+ if application.is_already_running() or application.is_data_path_missing():
75 sys.exit()
76 # Remove/convert obsolete settings.
77 Settings().remove_obsolete_settings()
78
79=== modified file 'openlp/core/lib/__init__.py'
80--- openlp/core/lib/__init__.py 2016-08-12 19:16:12 +0000
81+++ openlp/core/lib/__init__.py 2016-10-17 19:43:08 +0000
82@@ -310,30 +310,23 @@
83
84 def create_separated_list(string_list):
85 """
86- Returns a string that represents a join of a list of strings with a localized separator. This function corresponds
87-
88- to QLocale::createSeparatedList which was introduced in Qt 4.8 and implements the algorithm from
89- http://www.unicode.org/reports/tr35/#ListPatterns
90-
91- :param string_list: List of unicode strings
92+ Returns a string that represents a join of a list of strings with a localized separator.
93+ Localized separation will be done via the translate() function by the translators.
94+
95+ :param string_list: List of unicode strings
96+ :return: Formatted string
97 """
98- if LooseVersion(Qt.PYQT_VERSION_STR) >= LooseVersion('4.9') and LooseVersion(Qt.qVersion()) >= LooseVersion('4.8'):
99- return QtCore.QLocale().createSeparatedList(string_list)
100- if not string_list:
101- return ''
102- elif len(string_list) == 1:
103- return string_list[0]
104- # TODO: Verify mocking of translate() test before conversion
105- elif len(string_list) == 2:
106- return translate('OpenLP.core.lib', '%s and %s',
107- 'Locale list separator: 2 items') % (string_list[0], string_list[1])
108+ list_length = len(string_list)
109+ if list_length == 1:
110+ list_to_string = string_list[0]
111+ elif list_length == 2:
112+ list_to_string = translate('OpenLP.core.lib', '{one} and {two}').format(one=string_list[0], two=string_list[1])
113+ elif list_length > 2:
114+ list_to_string = translate('OpenLP.core.lib', '{first}, and {last}').format(first=', '.join(string_list[:-1]),
115+ last=string_list[-1])
116 else:
117- merged = translate('OpenLP.core.lib', '%s, and %s',
118- 'Locale list separator: end') % (string_list[-2], string_list[-1])
119- for index in reversed(list(range(1, len(string_list) - 2))):
120- merged = translate('OpenLP.core.lib', '%s, %s',
121- 'Locale list separator: middle') % (string_list[index], merged)
122- return translate('OpenLP.core.lib', '%s, %s', 'Locale list separator: start') % (string_list[0], merged)
123+ list_to_string = ''
124+ return list_to_string
125
126
127 from .exceptions import ValidationError
128
129=== modified file 'openlp/core/ui/advancedtab.py'
130--- openlp/core/ui/advancedtab.py 2016-08-11 22:12:50 +0000
131+++ openlp/core/ui/advancedtab.py 2016-10-17 19:43:08 +0000
132@@ -397,27 +397,6 @@
133 self.data_directory_cancel_button.hide()
134 # Since data location can be changed, make sure the path is present.
135 self.current_data_path = AppLocation.get_data_path()
136- if not os.path.exists(self.current_data_path):
137- log.error('Data path not found {path}'.format(path=self.current_data_path))
138- answer = QtWidgets.QMessageBox.critical(
139- self, translate('OpenLP.AdvancedTab', 'Data Directory Error'),
140- translate('OpenLP.AdvancedTab', 'OpenLP data directory was not found\n\n{path}\n\n'
141- 'This data directory was previously changed from the OpenLP '
142- 'default location. If the new location was on removable '
143- 'media, that media needs to be made available.\n\n'
144- 'Click "No" to stop loading OpenLP. allowing you to fix the the problem.\n\n'
145- 'Click "Yes" to reset the data directory to the default '
146- 'location.').format(path=self.current_data_path),
147- QtWidgets.QMessageBox.StandardButtons(QtWidgets.QMessageBox.Yes | QtWidgets.QMessageBox.No),
148- QtWidgets.QMessageBox.No)
149- if answer == QtWidgets.QMessageBox.No:
150- log.info('User requested termination')
151- self.main_window.clean_up()
152- sys.exit()
153- # Set data location to default.
154- settings.remove('advanced/data path')
155- self.current_data_path = AppLocation.get_data_path()
156- log.warning('User requested data path set to default {path}'.format(path=self.current_data_path))
157 self.data_directory_label.setText(os.path.abspath(self.current_data_path))
158 # Don't allow data directory move if running portable.
159 if settings.value('advanced/is portable'):
160
161=== modified file 'openlp/core/ui/lib/treewidgetwithdnd.py'
162--- openlp/core/ui/lib/treewidgetwithdnd.py 2016-04-17 19:32:15 +0000
163+++ openlp/core/ui/lib/treewidgetwithdnd.py 2016-10-17 19:43:08 +0000
164@@ -26,7 +26,7 @@
165
166 from PyQt5 import QtCore, QtGui, QtWidgets
167
168-from openlp.core.common import Registry
169+from openlp.core.common import Registry, is_win
170
171
172 class TreeWidgetWithDnD(QtWidgets.QTreeWidget):
173@@ -108,6 +108,11 @@
174
175 :param event: Handle of the event pint passed
176 """
177+ # If we are on Windows, OpenLP window will not be set on top. For example, user can drag images to Library and
178+ # the folder stays on top of the group creation box. This piece of code fixes this issue.
179+ if is_win():
180+ self.setWindowState(self.windowState() & ~QtCore.Qt.WindowMinimized | QtCore.Qt.WindowActive)
181+ self.setWindowState(QtCore.Qt.WindowNoState)
182 if event.mimeData().hasUrls():
183 event.setDropAction(QtCore.Qt.CopyAction)
184 event.accept()
185
186=== modified file 'openlp/core/ui/slidecontroller.py'
187--- openlp/core/ui/slidecontroller.py 2016-08-10 18:50:40 +0000
188+++ openlp/core/ui/slidecontroller.py 2016-10-17 19:43:08 +0000
189@@ -732,8 +732,10 @@
190 # Reset the button
191 self.play_slides_once.setChecked(False)
192 self.play_slides_once.setIcon(build_icon(':/media/media_time.png'))
193+ self.play_slides_once.setText(UiStrings().PlaySlidesToEnd)
194 self.play_slides_loop.setChecked(False)
195 self.play_slides_loop.setIcon(build_icon(':/media/media_time.png'))
196+ self.play_slides_loop.setText(UiStrings().PlaySlidesInLoop)
197 if item.is_text():
198 if (Settings().value(self.main_window.songs_settings_section + '/display songbar') and
199 not self.song_menu.menu().isEmpty()):
200
201=== modified file 'tests/functional/openlp_core_lib/test_lib.py'
202--- tests/functional/openlp_core_lib/test_lib.py 2016-08-09 06:24:04 +0000
203+++ tests/functional/openlp_core_lib/test_lib.py 2016-10-17 19:43:08 +0000
204@@ -689,50 +689,38 @@
205 """
206 Test the create_separated_list function with a list consisting of only one entry
207 """
208- with patch('openlp.core.lib.Qt') as mocked_qt:
209- # GIVEN: A list with a string and the mocked Qt module.
210- mocked_qt.PYQT_VERSION_STR = '4.8'
211- mocked_qt.qVersion.return_value = '4.7'
212- string_list = ['Author 1']
213-
214- # WHEN: We get a string build from the entries it the list and a separator.
215- string_result = create_separated_list(string_list)
216-
217- # THEN: We should have "Author 1"
218- assert string_result == 'Author 1', 'The string should be u\'Author 1\'.'
219+ # GIVEN: A list with a string.
220+ string_list = ['Author 1']
221+
222+ # WHEN: We get a string build from the entries it the list and a separator.
223+ string_result = create_separated_list(string_list)
224+
225+ # THEN: We should have "Author 1"
226+ assert string_result == 'Author 1', 'The string should be u\'Author 1\'.'
227
228 def test_create_separated_list_with_two_items(self):
229 """
230 Test the create_separated_list function with a list of two entries
231 """
232- with patch('openlp.core.lib.Qt') as mocked_qt, patch('openlp.core.lib.translate') as mocked_translate:
233- # GIVEN: A list of strings and the mocked Qt module.
234- mocked_qt.PYQT_VERSION_STR = '4.8'
235- mocked_qt.qVersion.return_value = '4.7'
236- mocked_translate.return_value = '%s and %s'
237- string_list = ['Author 1', 'Author 2']
238-
239- # WHEN: We get a string build from the entries it the list and a seperator.
240- string_result = create_separated_list(string_list)
241-
242- # THEN: We should have "Author 1 and Author 2"
243- assert string_result == 'Author 1 and Author 2', 'The string should be u\'Author 1 and Author 2\'.'
244+ # GIVEN: A list with two strings.
245+ string_list = ['Author 1', 'Author 2']
246+
247+ # WHEN: We get a string build from the entries it the list and a seperator.
248+ string_result = create_separated_list(string_list)
249+
250+ # THEN: We should have "Author 1 and Author 2"
251+ assert string_result == 'Author 1 and Author 2', 'The string should be u\'Author 1 and Author 2\'.'
252
253 def test_create_separated_list_with_three_items(self):
254 """
255 Test the create_separated_list function with a list of three items
256 """
257- with patch('openlp.core.lib.Qt') as mocked_qt, patch('openlp.core.lib.translate') as mocked_translate:
258- # GIVEN: A list with a string and the mocked Qt module.
259- mocked_qt.PYQT_VERSION_STR = '4.8'
260- mocked_qt.qVersion.return_value = '4.7'
261- # Always return the untranslated string.
262- mocked_translate.side_effect = lambda module, string_to_translate, comment: string_to_translate
263- string_list = ['Author 1', 'Author 2', 'Author 3']
264-
265- # WHEN: We get a string build from the entries it the list and a seperator.
266- string_result = create_separated_list(string_list)
267-
268- # THEN: We should have "Author 1, Author 2, and Author 3"
269- assert string_result == 'Author 1, Author 2, and Author 3', 'The string should be u\'Author 1, ' \
270- 'Author 2, and Author 3\'.'
271+ # GIVEN: A list with three strings.
272+ string_list = ['Author 1', 'Author 2', 'Author 3']
273+
274+ # WHEN: We get a string build from the entries it the list and a seperator.
275+ string_result = create_separated_list(string_list)
276+
277+ # THEN: We should have "Author 1, Author 2, and Author 3"
278+ assert string_result == 'Author 1, Author 2, and Author 3', 'The string should be u\'Author 1, ' \
279+ 'Author 2, and Author 3\'.'
280
281=== modified file 'tests/functional/openlp_plugins/bibles/test_swordimport.py'
282--- tests/functional/openlp_plugins/bibles/test_swordimport.py 2016-09-04 21:16:43 +0000
283+++ tests/functional/openlp_plugins/bibles/test_swordimport.py 2016-10-17 19:43:08 +0000
284@@ -70,8 +70,7 @@
285
286 @patch('openlp.plugins.bibles.lib.importers.sword.SwordBible.application')
287 @patch('openlp.plugins.bibles.lib.importers.sword.modules')
288- @patch('openlp.core.common.languages')
289- def test_simple_import(self, mocked_languages, mocked_pysword_modules, mocked_application):
290+ def test_simple_import(self, mocked_pysword_modules, mocked_application):
291 """
292 Test that a simple SWORD import works
293 """
294@@ -88,7 +87,7 @@
295 importer.create_verse = MagicMock()
296 importer.create_book = MagicMock()
297 importer.session = MagicMock()
298- mocked_languages.get_language.return_value = 'Danish'
299+ importer.get_language = MagicMock(return_value='Danish')
300 mocked_bible = MagicMock()
301 mocked_genesis = MagicMock()
302 mocked_genesis.name = 'Genesis'