openlp-core team mailing list archive

[Samuel Mehrbrodt <s.mehrbrodt@xxxxxxxxx>]

Samuel Mehrbrodt has proposed merging lp:~sam92/openlp/bug-1247493 into lp:openlp.

Requested reviews:
  OpenLP Core (openlp-core)
Related bugs:
  Bug #1247493 in OpenLP: "nightly build 2286 can't import Opensong NIV Bible"
  https://bugs.launchpad.net/openlp/+bug/1247493

For more details, see:
https://code.launchpad.net/~sam92/openlp/bug-1247493/+merge/216028

Fix bug 1247493: OpenSong Importer fails
-- 
https://code.launchpad.net/~sam92/openlp/bug-1247493/+merge/216028
Your team OpenLP Core is requested to review the proposed merge of lp:~sam92/openlp/bug-1247493 into lp:openlp.

=== modified file 'openlp/plugins/bibles/lib/opensong.py'
--- openlp/plugins/bibles/lib/opensong.py	2014-03-09 10:26:28 +0000
+++ openlp/plugins/bibles/lib/opensong.py	2014-04-16 08:28:49 +0000
@@ -73,13 +73,13 @@
         log.debug('Starting OpenSong import from "%s"' % self.filename)
         if not isinstance(self.filename, str):
             self.filename = str(self.filename, 'utf8')
-        file = None
+        import_file = None
         success = True
         try:
             # 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.
-            file = open(self.filename, 'r')
-            opensong = objectify.parse(file)
+            import_file = open(self.filename, 'rb')
+            opensong = objectify.parse(import_file)
             bible = opensong.getroot()
             language_id = self.get_language(bible_name)
             if not language_id:
@@ -93,7 +93,7 @@
                     log.error('Importing books from "%s" failed' % self.filename)
                     return False
                 book_details = BiblesResourcesDB.get_book_by_id(book_ref_id)
-                db_book = self.create_book(str(book.attrib['n']), book_ref_id, book_details['testament_id'])
+                db_book = self.create_book(book.attrib['n'], book_ref_id, book_details['testament_id'])
                 chapter_number = 0
                 for chapter in book.c:
                     if self.stop_import_flag:
@@ -122,8 +122,8 @@
                             verse_number += 1
                         self.create_verse(db_book.id, chapter_number, verse_number, self.get_text(verse))
                     self.wizard.increment_progress_bar(
-                        translate('BiblesPlugin.Opensong', 'Importing %s %s...',
-                                  'Importing <book name> <chapter>...')) % (db_book.name, chapter_number)
+                        translate('BiblesPlugin.Opensong', 'Importing %(bookname)s %(chapter)s...' %
+                                  {'bookname':db_book.name, 'chapter': chapter_number}))
                 self.session.commit()
             self.application.process_events()
         except etree.XMLSyntaxError as inst:
@@ -137,8 +137,8 @@
             log.exception('Loading Bible from OpenSong file failed')
             success = False
         finally:
-            if file:
-                file.close()
+            if import_file:
+                import_file.close()
         if self.stop_import_flag:
             return False
         else:

=== modified file 'tests/functional/openlp_plugins/songs/test_songbeamerimport.py'
--- tests/functional/openlp_plugins/songs/test_songbeamerimport.py	2014-04-14 18:33:34 +0000
+++ tests/functional/openlp_plugins/songs/test_songbeamerimport.py	2014-04-16 08:28:49 +0000
@@ -49,7 +49,8 @@
             ('4. Lobsingt seiner Treu´,\ndie immerdar neu,\nbis Er uns zur Herrlichket führet!\n\n', 'v')
         ],
         'song_book_name': 'Glaubenslieder I',
-        'song_number': "1"
+        'song_number': "1",
+        'authors': ['Carl Brockhaus', 'Johann Jakob Vetter']
     }
 }
 
@@ -140,6 +141,7 @@
                 add_verse_calls = SONG_TEST_DATA[song_file]['verses']
                 song_book_name = SONG_TEST_DATA[song_file]['song_book_name']
                 song_number = SONG_TEST_DATA[song_file]['song_number']
+                song_authors = SONG_TEST_DATA[song_file]['authors']
 
                 # THEN: do_import should return none, the song data should be as expected, and finish should have been
                 #       called.
@@ -153,6 +155,9 @@
                 if song_number:
                     self.assertEqual(importer.song_number, song_number, 'song_number for %s should be %s' %
                                                                          (song_file, song_number))
+                if song_authors:
+                    for author in importer.authors:
+                        self.assertIn(author, song_authors)
                 mocked_finish.assert_called_with()
 
     def check_verse_marks_test(self):

=== modified file 'tests/resources/songbeamersongs/Lobsinget dem Herrn.sng'
--- tests/resources/songbeamersongs/Lobsinget dem Herrn.sng	2013-10-06 22:07:49 +0000
+++ tests/resources/songbeamersongs/Lobsinget dem Herrn.sng	2014-04-16 08:28:49 +0000
@@ -1,5 +1,7 @@
 #LangCount=1
 #Title=GL 1 - Lobsinget dem Herrn
+#Author=Carl Brockhaus
+#Melody=Johann Jakob Vetter
 #Editor=SongBeamer 4.20
 #Version=3
 #Format=F/K//