openlp-core team mailing list archive

[Laura Ekstrand <laura@xxxxxxxxxxxxxx>]

Laura Ekstrand has proposed merging lp:~ldeks/openlp/ldeks-bugfix into lp:openlp.

Requested reviews:
  OpenLP Core (openlp-core)
Related bugs:
  Bug #1632567 in OpenLP: "OpenLP importer AttributeError: 'OldSong' object has no attribute 'book'"
  https://bugs.launchpad.net/openlp/+bug/1632567

For more details, see:
https://code.launchpad.net/~ldeks/openlp/ldeks-bugfix/+merge/312962

Fixes bug #1632567 "OpenLP importer AttributeError: 'OldSong' object has no attribute 'book'" by catching the exception and ignoring it (safely, I think).

Since I'm a new developer on OpenLP, I decided to go ahead and propose the merge without full testing to get a feel for your workflow.  Feel free to reject this.

I used this patch to workaround the problem successfully on my laptop running Fedora 25.  Now my songs import just fine.

I've verified that this solves the problem on my laptop, and nosetests pass.  However, the coverage as listed by nosetests in this part of the code is lacking.

I took a look at tests/functional/openlp_plugins/songs/test_openlpimport.py, but I wasn't quite sure how to test this change.

I also don't have an API for the Jenkins server, so I can't try it there, either.
-- 
Your team OpenLP Core is requested to review the proposed merge of lp:~ldeks/openlp/ldeks-bugfix into lp:openlp.

=== modified file 'openlp/plugins/songs/lib/importers/openlp.py'
--- openlp/plugins/songs/lib/importers/openlp.py	2016-05-27 08:13:14 +0000
+++ openlp/plugins/songs/lib/importers/openlp.py	2016-12-10 04:17:33 +0000
@@ -221,11 +221,14 @@
                     if not existing_book:
                         existing_book = Book.populate(name=entry.songbook.name, publisher=entry.songbook.publisher)
                     new_song.add_songbook_entry(existing_book, entry.entry)
-            elif song.book:
-                existing_book = self.manager.get_object_filtered(Book, Book.name == song.book.name)
-                if not existing_book:
-                    existing_book = Book.populate(name=song.book.name, publisher=song.book.publisher)
-                new_song.add_songbook_entry(existing_book, '')
+            else:
+                try:
+                    existing_book = self.manager.get_object_filtered(Book, Book.name == song.book.name)
+                    if not existing_book:
+                        existing_book = Book.populate(name=song.book.name, publisher=song.book.publisher)
+                    new_song.add_songbook_entry(existing_book, '')
+                except(AttributeError):
+                    pass
             # Find or create all the media files and add them to the new song object
             if has_media_files and song.media_files:
                 for media_file in song.media_files: