← Back to team overview

openlp-core team mailing list archive

[Merge] lp:~tomasgroth/openlp/bugfixes13 into lp:openlp

 

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

Requested reviews:
  Tim Bentley (trb143)
  Raoul Snyman (raoul-snyman)
Related bugs:
  Bug #1416888 in OpenLP: "Some disk errors are not being caught"
  https://bugs.launchpad.net/openlp/+bug/1416888
  Bug #1417033 in OpenLP: "Zefania bible imports all books as "Genesis" if book name is missing"
  https://bugs.launchpad.net/openlp/+bug/1417033
  Bug #1418212 in OpenLP: "Bible download from crosswalk fails because crosswalk has updated their layout"
  https://bugs.launchpad.net/openlp/+bug/1418212

For more details, see:
https://code.launchpad.net/~tomasgroth/openlp/bugfixes13/+merge/249261

Handle OSError exception when creating files in various places. Fixes bug 1416888.
Fix parsing biblestudytools.com. Fixes bug 1418212.
Make Zefania import guess book from number if name is unavailable. Fixes bug 1417033.
-- 
Your team OpenLP Core is subscribed to branch lp:openlp.
=== modified file 'openlp/core/lib/projector/constants.py'
--- openlp/core/lib/projector/constants.py	2015-01-22 19:46:04 +0000
+++ openlp/core/lib/projector/constants.py	2015-02-10 22:33:21 +0000
@@ -271,8 +271,8 @@
              E_PROXY_NOT_FOUND: translate('OpenLP.ProjectorConstants',
                                           'The proxy address set with setProxy() was not found'),
              E_PROXY_PROTOCOL: translate('OpenLP.ProjectorConstants',
-                                         'The connection negotiation with the proxy server because the response '
-                                         'from the proxy server could not be understood'),
+                                         'The connection negotiation with the proxy server failed because the '
+                                         'response from the proxy server could not be understood'),
              E_UNKNOWN_SOCKET_ERROR: translate('OpenLP.ProjectorConstants', 'An unidentified error occurred'),
              S_NOT_CONNECTED: translate('OpenLP.ProjectorConstants', 'Not connected'),
              S_CONNECTING: translate('OpenLP.ProjectorConstants', 'Connecting'),

=== modified file 'openlp/core/ui/mainwindow.py'
--- openlp/core/ui/mainwindow.py	2015-01-18 13:39:21 +0000
+++ openlp/core/ui/mainwindow.py	2015-02-10 22:33:21 +0000
@@ -989,15 +989,21 @@
         # Read the  temp file and output the user's CONF file with blanks to
         # make it more readable.
         temp_conf = open(temp_file, 'r')
-        export_conf = open(export_file_name, 'w')
-        for file_record in temp_conf:
-            # Get rid of any invalid entries.
-            if file_record.find('@Invalid()') == -1:
-                file_record = file_record.replace('%20', ' ')
-                export_conf.write(file_record)
-        temp_conf.close()
-        export_conf.close()
-        os.remove(temp_file)
+        try:
+            export_conf = open(export_file_name, 'w')
+            for file_record in temp_conf:
+                # Get rid of any invalid entries.
+                if file_record.find('@Invalid()') == -1:
+                    file_record = file_record.replace('%20', ' ')
+                    export_conf.write(file_record)
+            temp_conf.close()
+            export_conf.close()
+            os.remove(temp_file)
+        except OSError as ose:
+                QtGui.QMessageBox.critical(self, translate('OpenLP.MainWindow', 'Export setting error'),
+                                           translate('OpenLP.MainWindow', 'An error occurred while exporting the '
+                                                                          'settings: %s') % ose.strerror,
+                                           QtGui.QMessageBox.StandardButtons(QtGui.QMessageBox.Ok))
 
     def on_mode_default_item_clicked(self):
         """

=== modified file 'openlp/core/ui/servicemanager.py'
--- openlp/core/ui/servicemanager.py	2015-01-21 20:35:36 +0000
+++ openlp/core/ui/servicemanager.py	2015-02-10 22:33:21 +0000
@@ -601,6 +601,12 @@
                 shutil.copy(temp_file_name, path_file_name)
             except shutil.Error:
                 return self.save_file_as()
+            except OSError as ose:
+                QtGui.QMessageBox.critical(self, translate('OpenLP.ServiceManager', 'Error Saving File'),
+                                           translate('OpenLP.ServiceManager', 'An error occurred while writing the '
+                                                     'service file: %s') % ose.strerror,
+                                           QtGui.QMessageBox.StandardButtons(QtGui.QMessageBox.Ok))
+                success = False
             self.main_window.add_recent_file(path_file_name)
             self.set_modified(False)
         delete_file(temp_file_name)

=== modified file 'openlp/core/ui/thememanager.py'
--- openlp/core/ui/thememanager.py	2015-01-18 13:39:21 +0000
+++ openlp/core/ui/thememanager.py	2015-02-10 22:33:21 +0000
@@ -377,17 +377,11 @@
         self.application.set_busy_cursor()
         if path:
             Settings().setValue(self.settings_section + '/last directory export', path)
-            try:
-                self._export_theme(path, theme)
+            if self._export_theme(path, theme):
                 QtGui.QMessageBox.information(self,
                                               translate('OpenLP.ThemeManager', 'Theme Exported'),
                                               translate('OpenLP.ThemeManager',
                                                         'Your theme has been successfully exported.'))
-            except (IOError, OSError):
-                self.log_exception('Export Theme Failed')
-                critical_error_message_box(translate('OpenLP.ThemeManager', 'Theme Export Failed'),
-                                           translate('OpenLP.ThemeManager',
-                                                     'Your theme could not be exported due to an error.'))
         self.application.set_normal_cursor()
 
     def _export_theme(self, path, theme):
@@ -397,19 +391,24 @@
         :param theme: The name of the theme to be exported
         """
         theme_path = os.path.join(path, theme + '.otz')
+        theme_zip = None
         try:
             theme_zip = zipfile.ZipFile(theme_path, 'w')
             source = os.path.join(self.path, theme)
             for files in os.walk(source):
                 for name in files[2]:
                     theme_zip.write(os.path.join(source, name), os.path.join(theme, name))
-        except (IOError, OSError):
+            theme_zip.close()
+            return True
+        except OSError as ose:
+            self.log_exception('Export Theme Failed')
+            critical_error_message_box(translate('OpenLP.ThemeManager', 'Theme Export Failed'),
+                                       translate('OpenLP.ThemeManager', 'The theme export failed because this error '
+                                                                        'occurred: %s') % ose.strerror)
             if theme_zip:
                 theme_zip.close()
                 shutil.rmtree(theme_path, True)
-            raise
-        else:
-            theme_zip.close()
+            return False
 
     def on_import_theme(self, field=None):
         """

=== modified file 'openlp/plugins/bibles/lib/http.py'
--- openlp/plugins/bibles/lib/http.py	2015-01-18 13:39:21 +0000
+++ openlp/plugins/bibles/lib/http.py	2015-02-10 22:33:21 +0000
@@ -365,31 +365,20 @@
         if not soup:
             return None
         self.application.process_events()
-        html_verses = soup.find_all('span', 'versetext')
-        if not html_verses:
+        verses_div = soup.find_all('div', 'verse')
+        if not verses_div:
             log.error('No verses found in the CrossWalk response.')
             send_error_message('parse')
             return None
         verses = {}
-        for verse in html_verses:
+        for verse in verses_div:
             self.application.process_events()
-            verse_number = int(verse.contents[0].contents[0])
-            verse_text = ''
-            for part in verse.contents:
-                self.application.process_events()
-                if isinstance(part, NavigableString):
-                    verse_text += part
-                elif part and part.attrMap and \
-                        (part.attrMap['class'] == 'WordsOfChrist' or part.attrMap['class'] == 'strongs'):
-                    for subpart in part.contents:
-                        self.application.process_events()
-                        if isinstance(subpart, NavigableString):
-                            verse_text += subpart
-                        elif subpart and subpart.attrMap and subpart.attrMap['class'] == 'strongs':
-                            for subsub in subpart.contents:
-                                self.application.process_events()
-                                if isinstance(subsub, NavigableString):
-                                    verse_text += subsub
+            verse_number = int(verse.find('strong').contents[0])
+            verse_span = verse.find('span')
+            tags_to_remove = verse_span.find_all(['a', 'sup'])
+            for tag in tags_to_remove:
+                tag.decompose()
+            verse_text = verse_span.get_text()
             self.application.process_events()
             # Fix up leading and trailing spaces, multiple spaces, and spaces between text and , and .
             verse_text = verse_text.strip('\n\r\t ')
@@ -409,16 +398,13 @@
         soup = get_soup_for_bible_ref(chapter_url)
         if not soup:
             return None
-        content = soup.find('div', {'class': 'Body'})
-        content = content.find('ul', {'class': 'parent'})
+        content = soup.find_all(('h4', {'class': 'small-header'}))
         if not content:
             log.error('No books found in the Crosswalk response.')
             send_error_message('parse')
             return None
-        content = content.find_all('li')
         books = []
         for book in content:
-            book = book.find('a')
             books.append(book.contents[0])
         return books
 

=== modified file 'openlp/plugins/bibles/lib/osis.py'
--- openlp/plugins/bibles/lib/osis.py	2015-01-18 13:39:21 +0000
+++ openlp/plugins/bibles/lib/osis.py	2015-02-10 22:33:21 +0000
@@ -58,7 +58,7 @@
             # NOTE: We don't need to do any of the normal encoding detection here, because lxml does it's own encoding
             # detection, and the two mechanisms together interfere with each other.
             import_file = open(self.filename, 'rb')
-            osis_bible_tree = etree.parse(import_file)
+            osis_bible_tree = etree.parse(import_file, parser=etree.XMLParser(recover=True))
             namespace = {'ns': 'http://www.bibletechnologies.net/2003/OSIS/namespace'}
             # Find bible language
             language_id = None

=== modified file 'openlp/plugins/bibles/lib/zefania.py'
--- openlp/plugins/bibles/lib/zefania.py	2015-01-18 13:39:21 +0000
+++ openlp/plugins/bibles/lib/zefania.py	2015-02-10 22:33:21 +0000
@@ -57,7 +57,7 @@
             # NOTE: We don't need to do any of the normal encoding detection here, because lxml does it's own encoding
             # detection, and the two mechanisms together interfere with each other.
             import_file = open(self.filename, 'rb')
-            zefania_bible_tree = etree.parse(import_file)
+            zefania_bible_tree = etree.parse(import_file, parser=etree.XMLParser(recover=True))
             # Find bible language
             language_id = None
             language = zefania_bible_tree.xpath("/XMLBIBLE/INFORMATION/language/text()")
@@ -76,9 +76,19 @@
             etree.strip_elements(zefania_bible_tree, ('PROLOG', 'REMARK', 'CAPTION', 'MEDIA'), with_tail=False)
             xmlbible = zefania_bible_tree.getroot()
             for BIBLEBOOK in xmlbible:
-                book_ref_id = self.get_book_ref_id_by_name(str(BIBLEBOOK.get('bname')), num_books)
-                if not book_ref_id:
-                    book_ref_id = self.get_book_ref_id_by_localised_name(str(BIBLEBOOK.get('bname')))
+                if self.stop_import_flag:
+                    break
+                bname = BIBLEBOOK.get('bname')
+                bnumber = BIBLEBOOK.get('bnumber')
+                if not bname and not bnumber:
+                    continue
+                if bname:
+                    book_ref_id = self.get_book_ref_id_by_name(bname, num_books)
+                    if not book_ref_id:
+                        book_ref_id = self.get_book_ref_id_by_localised_name(bname)
+                else:
+                    log.debug('Could not find a name, will use number, basically a guess.')
+                    book_ref_id = int(bnumber)
                 if not book_ref_id:
                     log.error('Importing books from "%s" failed' % self.filename)
                     return False
@@ -94,7 +104,7 @@
                     self.wizard.increment_progress_bar(
                         translate('BiblesPlugin.Zefnia', 'Importing %(bookname)s %(chapter)s...' %
                                   {'bookname': db_book.name, 'chapter': chapter_number}))
-                self.session.commit()
+            self.session.commit()
             self.application.process_events()
         except Exception as e:
             critical_error_message_box(

=== modified file 'openlp/plugins/songs/forms/songexportform.py'
--- openlp/plugins/songs/forms/songexportform.py	2015-01-18 13:39:21 +0000
+++ openlp/plugins/songs/forms/songexportform.py	2015-02-10 22:33:21 +0000
@@ -244,12 +244,16 @@
             for song in self._find_list_widget_items(self.selected_list_widget)
         ]
         exporter = OpenLyricsExport(self, songs, self.directory_line_edit.text())
-        if exporter.do_export():
-            self.progress_label.setText(
-                translate('SongsPlugin.SongExportForm',
-                          'Finished export. To import these files use the <strong>OpenLyrics</strong> importer.'))
-        else:
-            self.progress_label.setText(translate('SongsPlugin.SongExportForm', 'Your song export failed.'))
+        try:
+            if exporter.do_export():
+                self.progress_label.setText(
+                    translate('SongsPlugin.SongExportForm',
+                              'Finished export. To import these files use the <strong>OpenLyrics</strong> importer.'))
+            else:
+                self.progress_label.setText(translate('SongsPlugin.SongExportForm', 'Your song export failed.'))
+        except OSError as ose:
+            self.progress_label.setText(translate('SongsPlugin.SongExportForm', 'Your song export failed because this '
+                                                  'error occurred: %s') % ose.strerror)
 
     def _find_list_widget_items(self, list_widget, text=''):
         """

=== modified file 'openlp/plugins/songusage/forms/songusagedetailform.py'
--- openlp/plugins/songusage/forms/songusagedetailform.py	2015-01-18 13:39:21 +0000
+++ openlp/plugins/songusage/forms/songusagedetailform.py	2015-02-10 22:33:21 +0000
@@ -27,6 +27,7 @@
 from sqlalchemy.sql import and_
 
 from openlp.core.common import RegistryProperties, Settings, check_directory_exists, translate
+from openlp.core.lib.ui import critical_error_message_box
 from openlp.plugins.songusage.lib.db import SongUsageItem
 from .songusagedetaildialog import Ui_SongUsageDetailDialog
 
@@ -104,8 +105,11 @@
                 translate('SongUsagePlugin.SongUsageDetailForm',
                           'Report \n%s \nhas been successfully created. ') % report_file_name
             )
-        except IOError:
+        except OSError as ose:
             log.exception('Failed to write out song usage records')
+            critical_error_message_box(translate('SongUsagePlugin.SongUsageDetailForm', 'Report Creation Failed'),
+                                       translate('SongUsagePlugin.SongUsageDetailForm',
+                                                 'An error occurred while creating the report: %s') % ose.strerror)
         finally:
             if file_handle:
                 file_handle.close()

=== modified file 'tests/functional/openlp_plugins/bibles/test_zefaniaimport.py'
--- tests/functional/openlp_plugins/bibles/test_zefaniaimport.py	2015-01-18 13:39:21 +0000
+++ tests/functional/openlp_plugins/bibles/test_zefaniaimport.py	2015-02-10 22:33:21 +0000
@@ -77,7 +77,6 @@
             mocked_import_wizard = MagicMock()
             importer = ZefaniaBible(mocked_manager, path='.', name='.', filename='')
             importer.wizard = mocked_import_wizard
-            importer.get_book_ref_id_by_name = MagicMock()
             importer.create_verse = MagicMock()
             importer.create_book = MagicMock()
             importer.session = MagicMock()
@@ -92,3 +91,34 @@
             self.assertTrue(importer.create_verse.called)
             for verse_tag, verse_text in test_data['verses']:
                 importer.create_verse.assert_any_call(importer.create_book().id, '1', verse_tag, verse_text)
+            importer.create_book.assert_any_call('Genesis', 1, 1)
+
+    def file_import_no_book_name_test(self):
+        """
+        Test the import of Zefania Bible file without book names
+        """
+        # GIVEN: Test files with a mocked out "manager", "import_wizard", and mocked functions
+        #        get_book_ref_id_by_name, create_verse, create_book, session and get_language.
+        result_file = open(os.path.join(TEST_PATH, 'rst.json'), 'rb')
+        test_data = json.loads(result_file.read().decode())
+        bible_file = 'zefania-rst.xml'
+        with patch('openlp.plugins.bibles.lib.zefania.ZefaniaBible.application'):
+            mocked_manager = MagicMock()
+            mocked_import_wizard = MagicMock()
+            importer = ZefaniaBible(mocked_manager, path='.', name='.', filename='')
+            importer.wizard = mocked_import_wizard
+            importer.create_verse = MagicMock()
+            importer.create_book = MagicMock()
+            importer.session = MagicMock()
+            importer.get_language = MagicMock()
+            importer.get_language.return_value = 'Russian'
+
+            # WHEN: Importing bible file
+            importer.filename = os.path.join(TEST_PATH, bible_file)
+            importer.do_import()
+
+            # THEN: The create_verse() method should have been called with each verse in the file.
+            self.assertTrue(importer.create_verse.called)
+            for verse_tag, verse_text in test_data['verses']:
+                importer.create_verse.assert_any_call(importer.create_book().id, '1', verse_tag, verse_text)
+            importer.create_book.assert_any_call('Exodus', 2, 1)

=== modified file 'tests/functional/openlp_plugins/songs/test_songselect.py'
--- tests/functional/openlp_plugins/songs/test_songselect.py	2015-01-18 13:39:21 +0000
+++ tests/functional/openlp_plugins/songs/test_songselect.py	2015-02-10 22:33:21 +0000
@@ -506,7 +506,6 @@
 
             # WHEN: We call the song importer
             song_import.do_import()
-            print(song_import.verses)
             # THEN: Song values should be equal to test values in setUp
             self.assertEquals(song_import.title, self.title, 'Song title should match')
             self.assertEquals(song_import.ccli_number, self.ccli_number, 'CCLI Song Number should match')

=== added file 'tests/resources/bibles/rst.json'
--- tests/resources/bibles/rst.json	1970-01-01 00:00:00 +0000
+++ tests/resources/bibles/rst.json	2015-02-10 22:33:21 +0000
@@ -0,0 +1,16 @@
+{
+    "book": "Exodus",
+    "chapter": 1,
+    "verses": [
+        [ "1", "Вот имена сынов Израилевых, которые вошли в Египет с Иаковом, вошли каждый с домом своим:" ],
+        [ "2", "Рувим, Симеон, Левий и Иуда," ],
+        [ "3", "Иссахар, Завулон и Вениамин," ],
+        [ "4", "Дан и Неффалим, Гад и Асир." ],
+        [ "5", "Всех же душ, происшедших от чресл Иакова, было семьдесят, а Иосиф был [уже] в Египте." ],
+        [ "6", "И умер Иосиф и все братья его и весь род их;" ],
+        [ "7", "а сыны Израилевы расплодились и размножились, и возросли и усилились чрезвычайно, и наполнилась ими земля та." ],
+        [ "8", "И восстал в Египте новый царь, который не знал Иосифа," ],
+        [ "9", "и сказал народу своему: вот, народ сынов Израилевых многочислен и сильнее нас;" ],
+        [ "10", "перехитрим же его, чтобы он не размножался; иначе, когда случится война, соединится и он с нашими неприятелями, и вооружится против нас, и выйдет из земли [нашей]." ]
+    ]
+}

=== added file 'tests/resources/bibles/zefania-rst.xml'
--- tests/resources/bibles/zefania-rst.xml	1970-01-01 00:00:00 +0000
+++ tests/resources/bibles/zefania-rst.xml	2015-02-10 22:33:21 +0000
@@ -0,0 +1,48 @@
+<?xml version="1.0" encoding="utf-8"?>
+<!--Visit the online documentation for Zefania XML Markup-->
+<!--http://bgfdb.de/zefaniaxml/bml/-->
+<!--Download another Zefania XML files from-->
+<!--http://sourceforge.net/projects/zefania-sharp-->
+<XMLBIBLE xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"; xsi:noNamespaceSchemaLocation="zef2005.xsd" version="2.0.1.18" status="v" biblename="Russian Synodal Translation" type="x-bible" revision="0">
+  <INFORMATION>
+    <title>Russian Synodal Translation</title>
+    <format>Zefania XML Bible Markup Language</format>
+    <date>2009-01-20</date>
+    <creator>Jens Grabner</creator>
+    <source>http://www.agape-biblia.org
+            http://www.crosswire.org/sword/modules/</source>
+    <language>RUS</language>
+    <publisher />
+    <identifier>RST</identifier>
+    <contributors>
+  1876 Russian Synodal Translation, 1956 Edition
+  The text was supplied by "Light in East Germany".</contributors>
+    <rights>
+    </rights>
+    <description>
+        "Light in East Germany"       Tel +49 711 83 30 57
+         Postfach 1340                Fax +49 711 83 13 51
+         7015 Korntal
+         Munchingen 1
+         Germany
+    </description>
+    <subject />
+    <type />
+    <coverage />
+  </INFORMATION>
+  <BIBLEBOOK bnumber="2">
+    <CHAPTER cnumber="1">
+      <CAPTION vref="1">Вторая книга Моисеева. Исход</CAPTION>
+      <VERS vnumber="1">Вот имена сынов Израилевых, которые вошли в Египет с Иаковом, вошли каждый с домом своим:</VERS>
+      <VERS vnumber="2">Рувим, Симеон, Левий и Иуда,</VERS>
+      <VERS vnumber="3">Иссахар, Завулон и Вениамин,</VERS>
+      <VERS vnumber="4">Дан и Неффалим, Гад и Асир.</VERS>
+      <VERS vnumber="5">Всех же душ, происшедших от чресл Иакова, было семьдесят, а Иосиф был [уже] в Египте.</VERS>
+      <VERS vnumber="6">И умер Иосиф и все братья его и весь род их;</VERS>
+      <VERS vnumber="7">а сыны Израилевы расплодились и размножились, и возросли и усилились чрезвычайно, и наполнилась ими земля та.</VERS>
+      <VERS vnumber="8">И восстал в Египте новый царь, который не знал Иосифа,</VERS>
+      <VERS vnumber="9">и сказал народу своему: вот, народ сынов Израилевых многочислен и сильнее нас;</VERS>
+      <VERS vnumber="10">перехитрим же его, чтобы он не размножался; иначе, когда случится война, соединится и он с нашими неприятелями, и вооружится против нас, и выйдет из земли [нашей].</VERS>
+    </CHAPTER>
+  </BIBLEBOOK>
+</XMLBIBLE>


References