Merge lp:~tomasgroth/openlp/24bugfix-backport1 into lp:openlp/2.4

[Tomas Groth]

Fix slide order change when splitting custom slides. Fixes bug 1554748.
Fix EasyWorship import issues with missing verses and traceback on unknown chars. Fixes bug 1553922 and 1547234.
Fix traceback in the bug-report dialog. Fixes bug 1554428.
Fix weird test bug in test_pluginmanager.py.
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.
Fix song tag detection. Fixes bug 1549549.
Fix a method call with too many parentheses, which fixes getting bible books from crosswalk.
Fix bug that prevents song book entries to be imported.

[Tomas Groth (tomasgroth)]

lp:~tomasgroth/openlp/24bugfix-backport1 (revision 2625)
[SUCCESS] https://ci.openlp.io/job/Branch-01-Pull/1475/
[SUCCESS] https://ci.openlp.io/job/Branch-02-Functional-Tests/1386/
[SUCCESS] https://ci.openlp.io/job/Branch-03-Interface-Tests/1324/
[SUCCESS] https://ci.openlp.io/job/Branch-04a-Windows_Functional_Tests/1126/
[SUCCESS] https://ci.openlp.io/job/Branch-04b-Windows_Interface_Tests/717/
[SUCCESS] https://ci.openlp.io/job/Branch-05a-Code_Analysis/784/
[SUCCESS] https://ci.openlp.io/job/Branch-05b-Test_Coverage/652/

[Tim Bentley (trb143)]

review: Approve

[Raoul Snyman (raoul-snyman)]

review: Approve

=== modified file 'openlp/core/ui/exceptionform.py'
--- openlp/core/ui/exceptionform.py	2016-01-09 16:26:14 +0000
+++ openlp/core/ui/exceptionform.py	2016-04-21 20:23:31 +0000
@@ -180,11 +180,13 @@
             if ':' in line:
                 exception = line.split('\n')[-1].split(':')[0]
         subject = 'Bug report: %s in %s' % (exception, source)
-        mail_to_url = QtCore.QUrlQuery('mailto:[e-mail]')
-        mail_to_url.addQueryItem('subject', subject)
-        mail_to_url.addQueryItem('body', self.report_text % content)
+        mail_urlquery = QtCore.QUrlQuery()
+        mail_urlquery.addQueryItem('subject', subject)
+        mail_urlquery.addQueryItem('body', self.report_text % content)
         if self.file_attachment:
-            mail_to_url.addQueryItem('attach', self.file_attachment)
+            mail_urlquery.addQueryItem('attach', self.file_attachment)
+        mail_to_url = QtCore.QUrl('mailto:[e-mail]')
+        mail_to_url.setQuery(mail_urlquery)
         QtGui.QDesktopServices.openUrl(mail_to_url)
 
     def on_description_updated(self):

=== modified file 'openlp/plugins/bibles/lib/http.py'
--- openlp/plugins/bibles/lib/http.py	2016-02-04 20:17:40 +0000
+++ openlp/plugins/bibles/lib/http.py	2016-04-21 20:23:31 +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-01-08 17:44:47 +0000
+++ openlp/plugins/bibles/lib/mediaitem.py	2016-04-21 20:23:31 +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-21 20:23:31 +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/custom/forms/editcustomform.py'
--- openlp/plugins/custom/forms/editcustomform.py	2016-01-09 16:26:14 +0000
+++ openlp/plugins/custom/forms/editcustomform.py	2016-04-21 20:23:31 +0000
@@ -198,6 +198,7 @@
             # Insert all slides to make the old_slides list complete.
             for slide in slides:
                 old_slides.insert(old_row, slide)
+                old_row += 1
             self.slide_list_view.addItems(old_slides)
         self.slide_list_view.repaint()
 

=== modified file 'openlp/plugins/songs/lib/__init__.py'
--- openlp/plugins/songs/lib/__init__.py	2015-12-31 22:46:06 +0000
+++ openlp/plugins/songs/lib/__init__.py	2016-04-21 20:23:31 +0000
@@ -256,6 +256,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):
@@ -271,7 +272,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/easyworship.py'
--- openlp/plugins/songs/lib/importers/easyworship.py	2016-01-31 19:36:54 +0000
+++ openlp/plugins/songs/lib/importers/easyworship.py	2016-04-21 20:23:31 +0000
@@ -289,40 +289,45 @@
             for i in range(rec_count):
                 if self.stop_import_flag:
                     break
-                raw_record = db_file.read(record_size)
-                self.fields = self.record_structure.unpack(raw_record)
-                self.set_defaults()
-                self.title = self.get_field(fi_title).decode(self.encoding)
-                # Get remaining fields.
-                copy = self.get_field(fi_copy)
-                admin = self.get_field(fi_admin)
-                ccli = self.get_field(fi_ccli)
-                authors = self.get_field(fi_author)
-                words = self.get_field(fi_words)
-                if copy:
-                    self.copyright = copy.decode(self.encoding)
-                if admin:
+                try:
+                    raw_record = db_file.read(record_size)
+                    self.fields = self.record_structure.unpack(raw_record)
+                    self.set_defaults()
+                    self.title = self.get_field(fi_title).decode(self.encoding)
+                    # Get remaining fields.
+                    copy = self.get_field(fi_copy)
+                    admin = self.get_field(fi_admin)
+                    ccli = self.get_field(fi_ccli)
+                    authors = self.get_field(fi_author)
+                    words = self.get_field(fi_words)
                     if copy:
-                        self.copyright += ', '
-                    self.copyright += translate('SongsPlugin.EasyWorshipSongImport',
-                                                'Administered by %s') % admin.decode(self.encoding)
-                if ccli:
-                    self.ccli_number = ccli.decode(self.encoding)
-                if authors:
-                    authors = authors.decode(self.encoding)
-                else:
-                    authors = ''
-                # Set the SongImport object members.
-                self.set_song_import_object(authors, words)
-                if self.stop_import_flag:
-                    break
-                if self.entry_error_log:
+                        self.copyright = copy.decode(self.encoding)
+                    if admin:
+                        if copy:
+                            self.copyright += ', '
+                        self.copyright += translate('SongsPlugin.EasyWorshipSongImport',
+                                                    'Administered by %s') % admin.decode(self.encoding)
+                    if ccli:
+                        self.ccli_number = ccli.decode(self.encoding)
+                    if authors:
+                        authors = authors.decode(self.encoding)
+                    else:
+                        authors = ''
+                    # Set the SongImport object members.
+                    self.set_song_import_object(authors, words)
+                    if self.stop_import_flag:
+                        break
+                    if self.entry_error_log:
+                        self.log_error(self.import_source,
+                                       translate('SongsPlugin.EasyWorshipSongImport', '"%s" could not be imported. %s')
+                                       % (self.title, self.entry_error_log))
+                        self.entry_error_log = ''
+                    elif not self.finish():
+                        self.log_error(self.import_source)
+                except Exception as e:
                     self.log_error(self.import_source,
                                    translate('SongsPlugin.EasyWorshipSongImport', '"%s" could not be imported. %s')
-                                   % (self.title, self.entry_error_log))
-                    self.entry_error_log = ''
-                elif not self.finish():
-                    self.log_error(self.import_source)
+                                   % (self.title, e))
         db_file.close()
         self.memo_file.close()
 
@@ -368,7 +373,7 @@
                 first_line_is_tag = False
                 # EW tags: verse, chorus, pre-chorus, bridge, tag,
                 # intro, ending, slide
-                for tag in VerseType.tags + ['tag', 'slide']:
+                for tag in VerseType.names + ['tag', 'slide', 'end']:
                     tag = tag.lower()
                     ew_tag = verse_split[0].strip().lower()
                     if ew_tag.startswith(tag):
@@ -390,6 +395,9 @@
                         if not number_found:
                             verse_type += '1'
                         break
+                # If the verse only consist of the tag-line, add an empty line to create an empty slide
+                if first_line_is_tag and len(verse_split) == 1:
+                    verse_split.append("")
                 self.add_verse(verse_split[-1].strip() if first_line_is_tag else verse, verse_type)
         if len(self.comments) > 5:
             self.comments += str(translate('SongsPlugin.EasyWorshipSongImport',

=== modified file 'openlp/plugins/songs/lib/importers/songimport.py'
--- openlp/plugins/songs/lib/importers/songimport.py	2015-12-31 22:46:06 +0000
+++ openlp/plugins/songs/lib/importers/songimport.py	2016-04-21 20:23:31 +0000
@@ -371,7 +371,7 @@
             song_book = self.manager.get_object_filtered(Book, Book.name == self.song_book_name)
             if song_book is None:
                 song_book = Book.populate(name=self.song_book_name, publisher=self.song_book_pub)
-            song.book = song_book
+            song.add_songbook_entry(song_book, song.song_number)
         for topic_text in self.topics:
             if not topic_text:
                 continue

=== modified file 'tests/functional/__init__.py'
--- tests/functional/__init__.py	2015-12-31 22:46:06 +0000
+++ tests/functional/__init__.py	2016-04-21 20:23:31 +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-21 20:23:31 +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)

=== modified file 'tests/interfaces/openlp_core_lib/test_pluginmanager.py'
--- tests/interfaces/openlp_core_lib/test_pluginmanager.py	2015-12-31 22:46:06 +0000
+++ tests/interfaces/openlp_core_lib/test_pluginmanager.py	2016-04-21 20:23:31 +0000
@@ -32,7 +32,7 @@
 
 from openlp.core.common import Registry, Settings
 from openlp.core.lib.pluginmanager import PluginManager
-from tests.interfaces import MagicMock
+from tests.interfaces import MagicMock, patch
 from tests.helpers.testmixin import TestMixin
 
 
@@ -45,13 +45,12 @@
         """
         Some pre-test setup required.
         """
-        Settings.setDefaultFormat(Settings.IniFormat)
+        self.setup_application()
         self.build_settings()
         self.temp_dir = mkdtemp('openlp')
         Settings().setValue('advanced/data path', self.temp_dir)
         Registry.create()
         Registry().register('service_list', MagicMock())
-        self.setup_application()
         self.main_window = QtWidgets.QMainWindow()
         Registry().register('main_window', self.main_window)
 
@@ -64,7 +63,13 @@
         gc.collect()
         shutil.rmtree(self.temp_dir)
 
-    def find_plugins_test(self):
+    @patch('openlp.plugins.songusage.lib.db.init_schema')
+    @patch('openlp.plugins.songs.lib.db.init_schema')
+    @patch('openlp.plugins.images.lib.db.init_schema')
+    @patch('openlp.plugins.custom.lib.db.init_schema')
+    @patch('openlp.plugins.alerts.lib.db.init_schema')
+    @patch('openlp.plugins.bibles.lib.db.init_schema')
+    def find_plugins_test(self, mocked_is1, mocked_is2, mocked_is3, mocked_is4, mocked_is5, mocked_is6):
         """
         Test the find_plugins() method to ensure it imports the correct plugins
         """

=== modified file 'tests/interfaces/openlp_plugins/custom/forms/test_customform.py'
--- tests/interfaces/openlp_plugins/custom/forms/test_customform.py	2015-12-31 22:46:06 +0000
+++ tests/interfaces/openlp_plugins/custom/forms/test_customform.py	2016-04-21 20:23:31 +0000
@@ -128,3 +128,19 @@
             # THEN: The validate method should have returned False.
             assert not result, 'The _validate() method should have retured False'
             mocked_critical_error_message_box.assert_called_with(message='You need to add at least one slide.')
+
+    def update_slide_list_test(self):
+        """
+        Test the update_slide_list() method
+        """
+        # GIVEN: Mocked slide_list_view with a slide with 3 lines
+        self.form.slide_list_view = MagicMock()
+        self.form.slide_list_view.count.return_value = 1
+        self.form.slide_list_view.currentRow.return_value = 0
+        self.form.slide_list_view.item.return_value = MagicMock(return_value='1st Slide\n2nd Slide\n3rd Slide')
+
+        # WHEN: updating the slide by splitting the lines into slides
+        self.form.update_slide_list(['1st Slide', '2nd Slide', '3rd Slide'])
+
+        # THEN: The slides should be created in correct order
+        self.form.slide_list_view.addItems.assert_called_with(['1st Slide', '2nd Slide', '3rd Slide'])