← Back to team overview

openlp-core team mailing list archive

Re: [Merge] lp:~gtalent/openlp/easyworship6 into lp:openlp

 

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.


References