Review: Approve
Generally I think it looks good! Just a few minor fixes.
Diff comments:
>
> === modified file 'openlp/plugins/songs/lib/importers/easyworship.py'
> --- openlp/plugins/songs/lib/importers/easyworship.py 2016-12-31 11:01:36
> +0000
> +++ openlp/plugins/songs/lib/importers/easyworship.py 2017-03-29 22:07:35
> +0000
> @@ -337,6 +340,69 @@
> db_file.close()
> self.memo_file.close()
>
> + def import_sqlite_db(self):
> + """
> + Import the songs from an EasyWorship 6 SQLite database
> + """
> + songs_db_path = self.import_source + "/Databases/Data/Songs.db"
> + song_words_db_path = self.import_source +
> "/Databases/Data/SongWords.db"
> + # check to see if needed files are there
Please do not hardcode paths. If the db files are copied to a different
computer without the EW6 folders, then it will not be possible to import the
DB. If you want to help the user find the DB, maybe you should put a help text
like for the VideoPsalm import.
> + if not os.path.isfile(songs_db_path):
> + self.log_error(songs_db_path,
> translate('SongsPlugin.EasyWorshipSongImport',
> + 'This file does not
> exist.'))
> + return
> + if not os.path.isfile(song_words_db_path):
> + self.log_error(song_words_db_path,
> translate('SongsPlugin.EasyWorshipSongImport',
> + 'Could not find the
> "Songs.MB" file. It must be in the same '
> + 'folder as the
> "Songs.DB" file.'))
> + return
> + # get database handles
> + songs_conn = sqlite3.connect(songs_db_path)
> + words_conn = sqlite3.connect(song_words_db_path)
> + if songs_conn is None or words_conn is None:
> + self.log_error(self.import_source,
> translate('SongsPlugin.EasyWorshipSongImport',
> + 'This is not a
> valid Easy Worship 6 database.'))
> + songs_conn.close()
> + words_conn.close()
> + return
> + songs_db = songs_conn.cursor()
> + words_db = words_conn.cursor()
> + if songs_conn is None or words_conn is None:
> + self.log_error(self.import_source,
> translate('SongsPlugin.EasyWorshipSongImport',
> + 'This is not a
> valid Easy Worship 6 database.'))
> + songs_conn.close()
> + words_conn.close()
> + return
> +
Please remove empty lines in methods.
> + # Take a stab at how text is encoded
> + self.encoding = 'cp1252'
> + self.encoding = retrieve_windows_encoding(self.encoding)
> + if not self.encoding:
> + log.debug('No encoding set.')
> + return
> +
> + # import songs
> + songs = songs_db.execute('SELECT
> rowid,title,author,copyright,vendor_id FROM song;')
> + for song in songs:
> + song_id = song[0]
> + # keep extra copy of title for error message because error check
> clears it
> + self.title = title = song[1]
> + self.author = song[2]
> + self.copyright = song[3]
> + self.ccli_number = song[4]
> + words = words_db.execute('SELECT words FROM word WHERE song_id =
> ?;', (song_id,))
> + self.set_song_import_object(self.author,
> words.fetchone()[0].encode())
> + if not self.finish():
> + self.log_error(self.import_source,
> + translate('SongsPlugin.EasyWorshipSongImport',
> + '"{title}" could not be imported.
> {entry}').
> + format(title=title,
> entry=self.entry_error_log))
> +
> + # close database handles
> + songs_conn.close()
> + words_conn.close()
> + return
> +
> def set_song_import_object(self, authors, words):
> """
> Set the SongImport object members.
--
https://code.launchpad.net/~gtalent/openlp/easyworship6/+merge/321381
Your team OpenLP Core is subscribed to branch lp:openlp.
_______________________________________________
Mailing list: https://launchpad.net/~openlp-core
Post to : [email protected]
Unsubscribe : https://launchpad.net/~openlp-core
More help : https://help.launchpad.net/ListHelp
