openlp-core team mailing list archive

[Tim Bentley <tim.bentley@xxxxxxxxx>]

Review: Needs Fixing

Question and no tests Sorry!

Diff comments:

> === modified file 'openlp/plugins/songs/lib/importers/openlp.py'
> --- openlp/plugins/songs/lib/importers/openlp.py	2016-12-31 11:01:36 +0000
> +++ openlp/plugins/songs/lib/importers/openlp.py	2017-01-08 22:10:23 +0000
> @@ -225,7 +225,13 @@
>                  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, '')
> +                # Get the song_number from "songs" table "song_number" field.
> +                # If there's a number, add it to the song, otherwise it will be "".
> +                existing_number = song.song_number

Why not test song.song_number instead of using an extra field.?

> +                if existing_number:
> +                    new_song.add_songbook_entry(existing_book, existing_number)
> +                else:
> +                    new_song.add_songbook_entry(existing_book, "")
>              # 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:


-- 
https://code.launchpad.net/~suutari-olli/openlp/fix-openlp-importer/+merge/314296
Your team OpenLP Core is subscribed to branch lp:openlp.