← Back to team overview

openlp-core team mailing list archive

[Merge] lp:~tomasgroth/openlp/25bugfixes2 into lp:openlp

 

Tomas Groth has proposed merging lp:~tomasgroth/openlp/25bugfixes2 into lp:openlp.

Requested reviews:
  Raoul Snyman (raoul-snyman)
Related bugs:
  Bug #1545072 in OpenLP: "Translation issues"
  https://bugs.launchpad.net/openlp/+bug/1545072
  Bug #1549549 in OpenLP: "import OpenSong songs with problem"
  https://bugs.launchpad.net/openlp/+bug/1549549
  Bug #1553863 in OpenLP: "If searching apocrypha book name while 2nd bible does not have it  = Traceback "
  https://bugs.launchpad.net/openlp/+bug/1553863
  Bug #1554554 in OpenLP: "translation on 639 text"
  https://bugs.launchpad.net/openlp/+bug/1554554

For more details, see:
https://code.launchpad.net/~tomasgroth/openlp/25bugfixes2/+merge/291465

Fix traceback when searching for book that doesn't exists in second bible. Fixes bug 1553863.
Set progress bar steps to number of chapters in zefania import.
Use standard button text in the FTW, if possible. Fixes bug 1554554.
Translation fixes. Fixes bug 1545072
Fix song tag detection. Fixes bug 1549549.
Fix a method call with too many parentheses, which fixes getting bible books from crosswalk.
-- 
Your team OpenLP Core is subscribed to branch lp:openlp.
=== modified file 'openlp/core/ui/firsttimewizard.py'
--- openlp/core/ui/firsttimewizard.py	2015-12-31 22:46:06 +0000
+++ openlp/core/ui/firsttimewizard.py	2016-04-10 20:39:44 +0000
@@ -249,10 +249,11 @@
         self.no_internet_text = translate('OpenLP.FirstTimeWizard',
                                           'No Internet connection was found. The First Time Wizard needs an Internet '
                                           'connection in order to be able to download sample songs, Bibles and themes.'
-                                          '  Click the Finish button now to start OpenLP with initial settings and '
+                                          '  Click the %s button now to start OpenLP with initial settings and '
                                           'no sample data.\n\nTo re-run the First Time Wizard and import this sample '
                                           'data at a later time, check your Internet connection and re-run this '
-                                          'wizard by selecting "Tools/Re-run First Time Wizard" from OpenLP.')
+                                          'wizard by selecting "Tools/Re-run First Time Wizard" from OpenLP.') % \
+            clean_button_text(first_time_wizard.buttonText(QtWidgets.QWizard.FinishButton))
         self.cancel_wizard_text = translate('OpenLP.FirstTimeWizard',
                                             '\n\nTo cancel the First Time Wizard completely (and not start OpenLP), '
                                             'click the %s button now.') % \
@@ -272,5 +273,7 @@
         self.progress_page.setSubTitle(translate('OpenLP.FirstTimeWizard', 'Please wait while resources are downloaded '
                                                                            'and OpenLP is configured.'))
         self.progress_label.setText(translate('OpenLP.FirstTimeWizard', 'Starting configuration process...'))
-        first_time_wizard.setButtonText(QtWidgets.QWizard.CustomButton1, translate('OpenLP.FirstTimeWizard', 'Finish'))
-        first_time_wizard.setButtonText(QtWidgets.QWizard.CustomButton2, translate('OpenLP.FirstTimeWizard', 'Cancel'))
+        first_time_wizard.setButtonText(QtWidgets.QWizard.CustomButton1,
+                                        clean_button_text(first_time_wizard.buttonText(QtWidgets.QWizard.FinishButton)))
+        first_time_wizard.setButtonText(QtWidgets.QWizard.CustomButton2,
+                                        clean_button_text(first_time_wizard.buttonText(QtWidgets.QWizard.CancelButton)))

=== modified file 'openlp/core/ui/thememanager.py'
--- openlp/core/ui/thememanager.py	2016-04-05 17:10:51 +0000
+++ openlp/core/ui/thememanager.py	2016-04-10 20:39:44 +0000
@@ -760,8 +760,8 @@
                     used_count = plugin.uses_theme(theme)
                     if used_count:
                         plugin_usage = "%s%s" % (plugin_usage, (translate('OpenLP.ThemeManager',
-                                                                          '%s time(s) by %s') %
-                                                                (used_count, plugin.name)))
+                                                                          '%(count)s time(s) by %(plugin)s') %
+                                                                {'count': used_count, 'plugin': plugin.name}))
                         plugin_usage = "%s\n" % plugin_usage
                 if plugin_usage:
                     critical_error_message_box(translate('OpenLP.ThemeManager', 'Unable to delete theme'),

=== modified file 'openlp/plugins/bibles/lib/http.py'
--- openlp/plugins/bibles/lib/http.py	2016-04-05 18:44:50 +0000
+++ openlp/plugins/bibles/lib/http.py	2016-04-10 20:39:44 +0000
@@ -504,7 +504,7 @@
         soup = get_soup_for_bible_ref(chapter_url)
         if not soup:
             return None
-        content = soup.find_all(('h4', {'class': 'small-header'}))
+        content = soup.find_all('h4', {'class': 'small-header'})
         if not content:
             log.error('No books found in the Crosswalk response.')
             send_error_message('parse')

=== modified file 'openlp/plugins/bibles/lib/mediaitem.py'
--- openlp/plugins/bibles/lib/mediaitem.py	2016-04-03 19:44:09 +0000
+++ openlp/plugins/bibles/lib/mediaitem.py	2016-04-10 20:39:44 +0000
@@ -764,6 +764,9 @@
                 except IndexError:
                     log.exception('The second_search_results does not have as many verses as the search_results.')
                     break
+                except TypeError:
+                    log.exception('The second_search_results does not have this book.')
+                    break
                 bible_text = '%s %d%s%d (%s, %s)' % (book, verse.chapter, verse_separator, verse.verse, version,
                                                      second_version)
             else:

=== modified file 'openlp/plugins/bibles/lib/zefania.py'
--- openlp/plugins/bibles/lib/zefania.py	2015-12-31 22:46:06 +0000
+++ openlp/plugins/bibles/lib/zefania.py	2016-04-10 20:39:44 +0000
@@ -70,7 +70,8 @@
                 log.error('Importing books from "%s" failed' % self.filename)
                 return False
             self.save_meta('language_id', language_id)
-            num_books = int(zefania_bible_tree.xpath("count(//BIBLEBOOK)"))
+            num_books = int(zefania_bible_tree.xpath('count(//BIBLEBOOK)'))
+            self.wizard.progress_bar.setMaximum(int(zefania_bible_tree.xpath('count(//CHAPTER)')))
             # Strip tags we don't use - keep content
             etree.strip_tags(zefania_bible_tree, ('STYLE', 'GRAM', 'NOTE', 'SUP', 'XREF'))
             # Strip tags we don't use - remove content

=== modified file 'openlp/plugins/songs/lib/__init__.py'
--- openlp/plugins/songs/lib/__init__.py	2016-04-05 17:30:20 +0000
+++ openlp/plugins/songs/lib/__init__.py	2016-04-10 20:39:44 +0000
@@ -255,6 +255,7 @@
         for num, translation in enumerate(VerseType.translated_names):
             if verse_name == translation.lower():
                 return num
+        return None
 
     @staticmethod
     def from_loose_input(verse_name, default=Other):
@@ -270,7 +271,7 @@
             if verse_index is None:
                 verse_index = VerseType.from_string(verse_name, default)
         elif len(verse_name) == 1:
-            verse_index = VerseType.from_translated_tag(verse_name, default)
+            verse_index = VerseType.from_translated_tag(verse_name, None)
             if verse_index is None:
                 verse_index = VerseType.from_tag(verse_name, default)
         else:

=== modified file 'openlp/plugins/songs/lib/importers/wordsofworship.py'
--- openlp/plugins/songs/lib/importers/wordsofworship.py	2015-12-31 22:46:06 +0000
+++ openlp/plugins/songs/lib/importers/wordsofworship.py	2016-04-10 20:39:44 +0000
@@ -107,9 +107,9 @@
                 song_data = open(source, 'rb')
                 if song_data.read(19).decode() != 'WoW File\nSong Words':
                     self.log_error(source,
-                                   str(translate('SongsPlugin.WordsofWorshipSongImport',
-                                                 'Invalid Words of Worship song file. Missing "%s" header.'
-                                                 % 'WoW File\\nSong Words')))
+                                   translate('SongsPlugin.WordsofWorshipSongImport',
+                                             'Invalid Words of Worship song file. Missing "%s" header.')
+                                   % 'WoW File\\nSong Words')
                     continue
                 # Seek to byte which stores number of blocks in the song
                 song_data.seek(56)
@@ -117,9 +117,9 @@
                 song_data.seek(66)
                 if song_data.read(16).decode() != 'CSongDoc::CBlock':
                     self.log_error(source,
-                                   str(translate('SongsPlugin.WordsofWorshipSongImport',
-                                                 'Invalid Words of Worship song file. Missing "%s" '
-                                                 'string.' % 'CSongDoc::CBlock')))
+                                   translate('SongsPlugin.WordsofWorshipSongImport',
+                                             'Invalid Words of Worship song file. Missing "%s" string.')
+                                   % 'CSongDoc::CBlock')
                     continue
                 # Seek to the beginning of the first block
                 song_data.seek(82)

=== modified file 'tests/functional/__init__.py'
--- tests/functional/__init__.py	2015-12-31 22:46:06 +0000
+++ tests/functional/__init__.py	2016-04-10 20:39:44 +0000
@@ -26,12 +26,12 @@
 from PyQt5 import QtWidgets
 
 if sys.version_info[1] >= 3:
-    from unittest.mock import ANY, MagicMock, patch, mock_open, call
+    from unittest.mock import ANY, MagicMock, patch, mock_open, call, PropertyMock
 else:
-    from mock import ANY, MagicMock, patch, mock_open, call
+    from mock import ANY, MagicMock, patch, mock_open, call, PropertyMock
 
 # Only one QApplication can be created. Use QtWidgets.QApplication.instance() when you need to "create" a  QApplication.
 application = QtWidgets.QApplication([])
 application.setApplicationName('OpenLP')
 
-__all__ = ['ANY', 'MagicMock', 'patch', 'mock_open', 'call', 'application']
+__all__ = ['ANY', 'MagicMock', 'patch', 'mock_open', 'call', 'application', 'PropertyMock']

=== modified file 'tests/functional/openlp_plugins/songs/test_lib.py'
--- tests/functional/openlp_plugins/songs/test_lib.py	2015-12-31 22:46:06 +0000
+++ tests/functional/openlp_plugins/songs/test_lib.py	2016-04-10 20:39:44 +0000
@@ -26,7 +26,7 @@
 
 from openlp.plugins.songs.lib import VerseType, clean_string, clean_title, strip_rtf
 from openlp.plugins.songs.lib.songcompare import songs_probably_equal, _remove_typos, _op_length
-from tests.functional import patch, MagicMock
+from tests.functional import patch, MagicMock, PropertyMock
 
 
 class TestLib(TestCase):
@@ -477,3 +477,27 @@
 
             # THEN: The result should be None
             self.assertIsNone(result, 'The result should be None, but was "%s"' % result)
+
+    @patch('openlp.plugins.songs.lib.VerseType.translated_tags', new_callable=PropertyMock, return_value=['x'])
+    def from_loose_input_with_invalid_input_test(self, mocked_translated_tags):
+        """
+        Test that the from_loose_input() method returns a sane default when passed an invalid tag and None as default.
+        """
+        # GIVEN: A mocked VerseType.translated_tags
+        # WHEN: We run the from_loose_input() method with an invalid verse type, we get the specified default back
+        result = VerseType.from_loose_input('m', None)
+
+        # THEN: The result should be None
+        self.assertIsNone(result, 'The result should be None, but was "%s"' % result)
+
+    @patch('openlp.plugins.songs.lib.VerseType.translated_tags', new_callable=PropertyMock, return_value=['x'])
+    def from_loose_input_with_valid_input_test(self, mocked_translated_tags):
+        """
+        Test that the from_loose_input() method returns valid output on valid input.
+        """
+        # GIVEN: A mocked VerseType.translated_tags
+        # WHEN: We run the from_loose_input() method with a valid verse type, we get the expected VerseType back
+        result = VerseType.from_loose_input('v')
+
+        # THEN: The result should be a Verse
+        self.assertEqual(result, VerseType.Verse, 'The result should be a verse, but was "%s"' % result)


Follow ups