trunk

.bzrignore

@@ -45,3 +45,4 @@ cover
 *.kdev4
 coverage
 tags
+output

openlp/core/ui/maindisplay.py

@@ -247,7 +247,7 @@ class MainDisplay(OpenLPMixin, Display, RegistryProperties):
         """
         Set up and build the output screen
         """
-        self.log_debug('Start MainDisplay setup (live = %s)' % self.is_live)
+        self.log_debug('Start MainDisplay setup (live = {islive})'.format(islive=self.is_live))
         self.screen = self.screens.current
         self.setVisible(False)
         Display.setup(self)
@@ -288,7 +288,9 @@ class MainDisplay(OpenLPMixin, Display, RegistryProperties):
             self.application.process_events()
         self.setGeometry(self.screen['size'])
         if animate:
-            self.frame.evaluateJavaScript('show_text("%s")' % slide.replace('\\', '\\\\').replace('\"', '\\\"'))
+            # NOTE: Verify this works with ''.format()
+            _text = slide.replace('\\', '\\\\').replace('\"', '\\\"')
+            self.frame.evaluateJavaScript('show_text("{text}")'.format(text=_text))
         else:
             # This exists for https://bugs.launchpad.net/openlp/+bug/1016843
             # For unknown reasons if evaluateJavaScript is called
@@ -309,10 +311,10 @@ class MainDisplay(OpenLPMixin, Display, RegistryProperties):
         text_prepared = expand_tags(html.escape(text)).replace('\\', '\\\\').replace('\"', '\\\"')
         if self.height() != self.screen['size'].height() or not self.isVisible():
             shrink = True
-            js = 'show_alert("%s", "%s")' % (text_prepared, 'top')
+            js = 'show_alert("{text}", "{top}")'.format(text=text_prepared, top='top')
         else:
             shrink = False
-            js = 'show_alert("%s", "")' % text_prepared
+            js = 'show_alert("{text}", "")'.format(text=text_prepared)
         height = self.frame.evaluateJavaScript(js)
         if shrink:
             if text:
@@ -368,7 +370,7 @@ class MainDisplay(OpenLPMixin, Display, RegistryProperties):
         """
         self.setGeometry(self.screen['size'])
         if image:
-            js = 'show_image("data:image/png;base64,%s");' % image
+            js = 'show_image("data:image/png;base64,{image}");'.format(image=image)
         else:
             js = 'show_image("");'
         self.frame.evaluateJavaScript(js)
@@ -492,7 +494,7 @@ class MainDisplay(OpenLPMixin, Display, RegistryProperties):
 
         :param mode: How the screen is to be hidden
         """
-        self.log_debug('hide_display mode = %d' % mode)
+        self.log_debug('hide_display mode = {mode:d}'.format(mode=mode))
         if self.screens.display_count == 1:
             # Only make visible if setting enabled.
             if not Settings().value('core/display on monitor'):

openlp/core/ui/mainwindow.py

@@ -622,11 +622,10 @@ class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow, RegistryProperties):
         :param version: The Version to be displayed.
         """
         log.debug('version_notice')
-        version_text = translate('OpenLP.MainWindow', 'Version %s of OpenLP is now available for download (you are '
+        version_text = translate('OpenLP.MainWindow', 'Version {new} of OpenLP is now available for download (you are '
-                                 'currently running version %s). \n\nYou can download the latest version from '
+                                 'currently running version {current}). \n\nYou can download the latest version from '
-                                 'http://openlp.org/.')
+                                 'http://openlp.org/.').format(new=version, current=get_application_version()[u'full'])
-        QtWidgets.QMessageBox.question(self, translate('OpenLP.MainWindow', 'OpenLP Version Updated'),
+        QtWidgets.QMessageBox.question(self, translate('OpenLP.MainWindow', 'OpenLP Version Updated'), version_text)
-                                       version_text % (version, get_application_version()[u'full']))
 
     def show(self):
         """
@@ -642,7 +641,7 @@ class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow, RegistryProperties):
             self.service_manager_contents.load_last_file()
         # This will store currently used layout preset so it remains enabled on next startup.
         # If any panel is enabled/disabled after preset is set, this setting is not saved.
-        view_mode = Settings().value('%s/view mode' % self.general_settings_section)
+        view_mode = Settings().value('{section}/view mode'.format(section=self.general_settings_section))
         if view_mode == 'default' and Settings().value('user interface/is preset layout'):
             self.mode_default_item.setChecked(True)
         elif view_mode == 'setup' and Settings().value('user interface/is preset layout'):
@@ -731,8 +730,8 @@ class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow, RegistryProperties):
         """
         settings = Settings()
         self.live_controller.main_display_set_background()
-        if settings.value('%s/screen blank' % self.general_settings_section):
+        if settings.value('{section}/screen blank'.format(section=self.general_settings_section)):
-            if settings.value('%s/blank warning' % self.general_settings_section):
+            if settings.value('{section}/blank warning'.format(section=self.general_settings_section)):
                 QtWidgets.QMessageBox.question(self, translate('OpenLP.MainWindow', 'OpenLP Main Display Blanked'),
                                                translate('OpenLP.MainWindow', 'The Main Display has been blanked out'))
 
@@ -924,9 +923,9 @@ class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow, RegistryProperties):
             try:
                 value = import_settings.value(section_key)
             except KeyError:
-                log.warning('The key "%s" does not exist (anymore), so it will be skipped.' % section_key)
+                log.warning('The key "{key}" does not exist (anymore), so it will be skipped.'.format(key=section_key))
             if value is not None:
-                settings.setValue('%s' % (section_key), value)
+                settings.setValue('{key}'.format(key=section_key), value)
         now = datetime.now()
         settings.beginGroup(self.header_section)
         settings.setValue('file_imported', import_file_name)
@@ -1003,9 +1002,9 @@ class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow, RegistryProperties):
                 key_value = settings.value(section_key)
             except KeyError:
                 QtWidgets.QMessageBox.critical(self, translate('OpenLP.MainWindow', 'Export setting error'),
-                                               translate('OpenLP.MainWindow', 'The key "%s" does not have a default '
+                                               translate('OpenLP.MainWindow', 'The key "{key}" does not have a default '
                                                                               'value so it will be skipped in this '
-                                                                              'export.') % section_key,
+                                                                              'export.').format(key=section_key),
                                                QtWidgets.QMessageBox.StandardButtons(QtWidgets.QMessageBox.Ok))
                 key_value = None
             if key_value is not None:
@@ -1027,8 +1026,9 @@ class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow, RegistryProperties):
             os.remove(temp_file)
         except OSError as ose:
                 QtWidgets.QMessageBox.critical(self, translate('OpenLP.MainWindow', 'Export setting error'),
-                                               translate('OpenLP.MainWindow', 'An error occurred while exporting the '
+                                               translate('OpenLP.MainWindow',
-                                                                              'settings: %s') % ose.strerror,
+                                                         'An error occurred while exporting the '
+                                                         'settings: {err}').format(err=ose.strerror),
                                                QtWidgets.QMessageBox.StandardButtons(QtWidgets.QMessageBox.Ok))
 
     def on_mode_default_item_clicked(self):
@@ -1061,7 +1061,7 @@ class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow, RegistryProperties):
         """
         if mode:
             settings = Settings()
-            settings.setValue('%s/view mode' % self.general_settings_section, mode)
+            settings.setValue('{section}/view mode'.format(section=self.general_settings_section), mode)
         self.media_manager_dock.setVisible(media)
         self.service_manager_dock.setVisible(service)
         self.theme_manager_dock.setVisible(theme)
@@ -1168,9 +1168,9 @@ class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow, RegistryProperties):
         :param file_name: The file name of the service file.
         """
         if modified:
-            title = '%s - %s*' % (UiStrings().OLPV2x, file_name)
+            title = '{title} - {name}*'.format(title=UiStrings().OLPV2x, name=file_name)
         else:
-            title = '%s - %s' % (UiStrings().OLPV2x, file_name)
+            title = '{title} - {name}'.format(title=UiStrings().OLPV2x, name=file_name)
         self.setWindowTitle(title)
 
     def show_status_message(self, message):
@@ -1183,8 +1183,9 @@ class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow, RegistryProperties):
         """
         Update the default theme indicator in the status bar
         """
-        self.default_theme_label.setText(translate('OpenLP.MainWindow', 'Default Theme: %s') %
+        theme_name = Settings().value('themes/global theme')
-                                         Settings().value('themes/global theme'))
+        self.default_theme_label.setText(translate('OpenLP.MainWindow',
+                                                   'Default Theme: {theme}').format(theme=theme_name))
 
     def toggle_media_manager(self):
         """
@@ -1331,7 +1332,8 @@ class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow, RegistryProperties):
         recent_files_to_display = existing_recent_files[0:recent_file_count]
         self.recent_files_menu.clear()
         for file_id, filename in enumerate(recent_files_to_display):
-            log.debug('Recent file name: %s', filename)
+            log.debug('Recent file name: {name}'.format(name=filename))
+            # TODO: Verify ''.format() before committing
             action = create_action(self, '', text='&%d %s' % (file_id + 1,
                                    os.path.splitext(os.path.basename(str(filename)))[0]), data=filename,
                                    triggers=self.service_manager_contents.on_recent_service_clicked)
@@ -1424,7 +1426,7 @@ class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow, RegistryProperties):
         """
         Change the data directory.
         """
-        log.info('Changing data path to %s' % self.new_data_path)
+        log.info('Changing data path to {newpath}'.format(newpath=self.new_data_path))
         old_data_path = str(AppLocation.get_data_path())
         # Copy OpenLP data to new location if requested.
         self.application.set_busy_cursor()
@@ -1432,17 +1434,17 @@ class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow, RegistryProperties):
             log.info('Copying data to new path')
             try:
                 self.show_status_message(
-                    translate('OpenLP.MainWindow', 'Copying OpenLP data to new data directory location - %s '
+                    translate('OpenLP.MainWindow', 'Copying OpenLP data to new data directory location - {path} '
-                              '- Please wait for copy to finish').replace('%s', self.new_data_path))
+                              '- Please wait for copy to finish').format(path=self.new_data_path))
                 dir_util.copy_tree(old_data_path, self.new_data_path)
                 log.info('Copy successful')
             except (IOError, os.error, DistutilsFileError) as why:
                 self.application.set_normal_cursor()
-                log.exception('Data copy failed %s' % str(why))
+                log.exception('Data copy failed {err}'.format(err=str(why)))
+                err_text = translate('OpenLP.MainWindow',
+                                     'OpenLP Data directory copy failed\n\n{err}').format(err=str(why)),
                 QtWidgets.QMessageBox.critical(self, translate('OpenLP.MainWindow', 'New Data Directory Error'),
-                                               translate('OpenLP.MainWindow',
+                                               err_text,
-                                                         'OpenLP Data directory copy failed\n\n%s').
-                                               replace('%s', str(why)),
                                                QtWidgets.QMessageBox.StandardButtons(QtWidgets.QMessageBox.Ok))
                 return False
         else:

openlp/plugins/songs/lib/db.py

@@ -383,7 +383,7 @@ def init_schema(url):
         # Use lazy='joined' to always load authors when the song is fetched from the database (bug 1366198)
         'authors': relation(Author, secondary=authors_songs_table, viewonly=True, lazy='joined'),
         'media_files': relation(MediaFile, backref='songs', order_by=media_files_table.c.weight),
-        'songbook_entries': relation(SongBookEntry, backref='song', cascade="all, delete-orphan"),
+        'songbook_entries': relation(SongBookEntry, backref='song', cascade='all, delete-orphan'),
         'topics': relation(Topic, backref='songs', secondary=songs_topics_table)
     })
     mapper(Topic, topics_table)

openlp/plugins/songs/lib/importers/openlp.py

@@ -51,7 +51,7 @@ class OpenLPSongImport(SongImport):
         :param manager: The song manager for the running OpenLP installation.
         :param kwargs: The database providing the data to import.
         """
-        SongImport.__init__(self, manager, **kwargs)
+        super(OpenLPSongImport, self).__init__(manager, **kwargs)
         self.source_session = None
 
     def do_import(self, progress_dialog=None):
@@ -63,49 +63,61 @@ class OpenLPSongImport(SongImport):
 
         class OldAuthor(BaseModel):
             """
-            Author model
+            Maps to the authors table
             """
             pass
 
         class OldBook(BaseModel):
             """
-            Book model
+            Maps to the songbooks table
             """
             pass
 
         class OldMediaFile(BaseModel):
             """
-            MediaFile model
+            Maps to the media_files table
             """
             pass
 
         class OldSong(BaseModel):
             """
-            Song model
+            Maps to the songs table
             """
             pass
 
         class OldTopic(BaseModel):
             """
-            Topic model
+            Maps to the topics table
+            """
+            pass
+
+        class OldSongBookEntry(BaseModel):
+            """
+            Maps to the songs_songbooks table
             """
             pass
 
         # Check the file type
-        if not self.import_source.endswith('.sqlite'):
+        if not isinstance(self.import_source, str) or not self.import_source.endswith('.sqlite'):
             self.log_error(self.import_source, translate('SongsPlugin.OpenLPSongImport',
                                                          'Not a valid OpenLP 2 song database.'))
             return
         self.import_source = 'sqlite:///%s' % self.import_source
-        # Load the db file
+        # Load the db file and reflect it
         engine = create_engine(self.import_source)
         source_meta = MetaData()
         source_meta.reflect(engine)
         self.source_session = scoped_session(sessionmaker(bind=engine))
+        # Run some checks to see which version of the database we have
         if 'media_files' in list(source_meta.tables.keys()):
             has_media_files = True
         else:
             has_media_files = False
+        if 'songs_songbooks' in list(source_meta.tables.keys()):
+            has_songs_books = True
+        else:
+            has_songs_books = False
+        # Load up the tabls and map them out
         source_authors_table = source_meta.tables['authors']
         source_song_books_table = source_meta.tables['song_books']
         source_songs_table = source_meta.tables['songs']
@@ -113,6 +125,7 @@ class OpenLPSongImport(SongImport):
         source_authors_songs_table = source_meta.tables['authors_songs']
         source_songs_topics_table = source_meta.tables['songs_topics']
         source_media_files_songs_table = None
+        # Set up media_files relations
         if has_media_files:
             source_media_files_table = source_meta.tables['media_files']
             source_media_files_songs_table = source_meta.tables.get('media_files_songs')
@@ -120,9 +133,15 @@ class OpenLPSongImport(SongImport):
                 class_mapper(OldMediaFile)
             except UnmappedClassError:
                 mapper(OldMediaFile, source_media_files_table)
+        if has_songs_books:
+            source_songs_songbooks_table = source_meta.tables['songs_songbooks']
+            try:
+                class_mapper(OldSongBookEntry)
+            except UnmappedClassError:
+                mapper(OldSongBookEntry, source_songs_songbooks_table, properties={'songbook': relation(OldBook)})
+        # Set up the songs relationships
         song_props = {
             'authors': relation(OldAuthor, backref='songs', secondary=source_authors_songs_table),
-            'book': relation(OldBook, backref='songs'),
             'topics': relation(OldTopic, backref='songs', secondary=source_songs_topics_table)
         }
         if has_media_files:
@@ -134,6 +153,11 @@ class OpenLPSongImport(SongImport):
                     relation(OldMediaFile, backref='songs',
                              foreign_keys=[source_media_files_table.c.song_id],
                              primaryjoin=source_songs_table.c.id == source_media_files_table.c.song_id)
+        if has_songs_books:
+            song_props['songbook_entries'] = relation(OldSongBookEntry, backref='song', cascade='all, delete-orphan')
+        else:
+            song_props['book'] = relation(OldBook, backref='songs')
+        # Map the rest of the tables
         try:
             class_mapper(OldAuthor)
         except UnmappedClassError:
@@ -163,44 +187,54 @@ class OpenLPSongImport(SongImport):
                 old_titles = song.search_title.split('@')
                 if len(old_titles) > 1:
                     new_song.alternate_title = old_titles[1]
-            # Values will be set when cleaning the song.
+            # Transfer the values to the new song object
             new_song.search_title = ''
             new_song.search_lyrics = ''
-            new_song.song_number = song.song_number
             new_song.lyrics = song.lyrics
             new_song.verse_order = song.verse_order
             new_song.copyright = song.copyright
             new_song.comments = song.comments
             new_song.theme_name = song.theme_name
             new_song.ccli_number = song.ccli_number
+            if hasattr(song, 'song_number') and song.song_number:
+                new_song.song_number = song.song_number
+            # Find or create all the authors and add them to the new song object
             for author in song.authors:
                 existing_author = self.manager.get_object_filtered(Author, Author.display_name == author.display_name)
-                if existing_author is None:
+                if not existing_author:
                     existing_author = Author.populate(
                         first_name=author.first_name,
                         last_name=author.last_name,
                         display_name=author.display_name)
                 new_song.add_author(existing_author)
-            if song.book:
+            # Find or create all the topics and add them to the new song object
-                existing_song_book = self.manager.get_object_filtered(Book, Book.name == song.book.name)
-                if existing_song_book is None:
-                    existing_song_book = Book.populate(name=song.book.name, publisher=song.book.publisher)
-                new_song.book = existing_song_book
             if song.topics:
                 for topic in song.topics:
                     existing_topic = self.manager.get_object_filtered(Topic, Topic.name == topic.name)
-                    if existing_topic is None:
+                    if not existing_topic:
                         existing_topic = Topic.populate(name=topic.name)
                     new_song.topics.append(existing_topic)
-            if has_media_files:
+            # Find or create all the songbooks and add them to the new song object
-                if song.media_files:
+            if has_songs_books and song.songbook_entries:
-                    for media_file in song.media_files:
+                for entry in song.songbook_entries:
-                        existing_media_file = self.manager.get_object_filtered(
+                    existing_book = self.manager.get_object_filtered(Book, Book.name == entry.songbook.name)
-                            MediaFile, MediaFile.file_name == media_file.file_name)
+                    if not existing_book:
-                        if existing_media_file:
+                        existing_book = Book.populate(name=entry.songbook.name, publisher=entry.songbook.publisher)
-                            new_song.media_files.append(existing_media_file)
+                    new_song.add_songbook_entry(existing_book, entry.entry)
-                        else:
+            elif song.book:
-                            new_song.media_files.append(MediaFile.populate(file_name=media_file.file_name))
+                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, '')
+            # 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:
+                    existing_media_file = self.manager.get_object_filtered(
+                        MediaFile, MediaFile.file_name == media_file.file_name)
+                    if existing_media_file:
+                        new_song.media_files.append(existing_media_file)
+                    else:
+                        new_song.media_files.append(MediaFile.populate(file_name=media_file.file_name))
             clean_song(self.manager, new_song)
             self.manager.save_object(new_song)
             if progress_dialog:

openlp/plugins/songs/lib/mediaitem.py

@@ -21,7 +21,6 @@
 ###############################################################################
 
 import logging
-import re
 import os
 import shutil
 
@@ -207,9 +206,11 @@ class SongMediaItem(MediaManagerItem):
             search_keywords = search_keywords.rpartition(' ')
             search_book = search_keywords[0] + '%'
             search_entry = search_keywords[2] + '%'
-            search_results = (self.plugin.manager.session.query(SongBookEntry)
+            search_results = (self.plugin.manager.session.query(SongBookEntry.entry, Book.name, Song.title, Song.id)
+                              .join(Song)
                               .join(Book)
-                              .filter(Book.name.like(search_book), SongBookEntry.entry.like(search_entry)).all())
+                              .filter(Book.name.like(search_book), SongBookEntry.entry.like(search_entry),
+                                      Song.temporary.is_(False)).all())
             self.display_results_book(search_results)
         elif search_type == SongSearch.Themes:
             log.debug('Theme Search')
@@ -313,23 +314,20 @@ class SongMediaItem(MediaManagerItem):
         """
         Display the song search results in the media manager list, grouped by book and entry
 
-        :param search_results: A list of db SongBookEntry objects
+        :param search_results: A tuple containing (songbook entry, book name, song title, song id)
         :return: None
         """
-        def get_songbook_key(songbook_entry):
+        def get_songbook_key(result):
             """Get the key to sort by"""
-            return (get_natural_key(songbook_entry.songbook.name), get_natural_key(songbook_entry.entry))
+            return (get_natural_key(result[1]), get_natural_key(result[0]), get_natural_key(result[2]))
 
         log.debug('display results Book')
         self.list_view.clear()
         search_results.sort(key=get_songbook_key)
-        for songbook_entry in search_results:
+        for result in search_results:
-            # Do not display temporary songs
+            song_detail = '%s #%s: %s' % (result[1], result[0], result[2])
-            if songbook_entry.song.temporary:
-                continue
-            song_detail = '%s #%s: %s' % (songbook_entry.songbook.name, songbook_entry.entry, songbook_entry.song.title)
             song_name = QtWidgets.QListWidgetItem(song_detail)
-            song_name.setData(QtCore.Qt.UserRole, songbook_entry.song.id)
+            song_name.setData(QtCore.Qt.UserRole, result[3])
             self.list_view.addItem(song_name)
 
     def display_results_topic(self, search_results):

tests/functional/openlp_core_lib/test_projectordb.py

@@ -28,7 +28,7 @@ PREREQUISITE: add_record() and get_all() functions validated.
 import os
 from unittest import TestCase
 
-from openlp.core.lib.projector.db import Projector, ProjectorDB, ProjectorSource
+from openlp.core.lib.projector.db import Manufacturer, Model, Projector, ProjectorDB, ProjectorSource
 
 from tests.functional import MagicMock, patch
 from tests.resources.projector.data import TEST_DB, TEST1_DATA, TEST2_DATA, TEST3_DATA
@@ -82,13 +82,13 @@ class TestProjectorDB(TestCase):
     """
     Test case for ProjectorDB
     """
-    def setUp(self):
+    @patch('openlp.core.lib.projector.db.init_url')
+    def setUp(self, mocked_init_url):
         """
         Set up anything necessary for all tests
         """
-        with patch('openlp.core.lib.projector.db.init_url') as mocked_init_url:
+        mocked_init_url.return_value = 'sqlite:///{db}'.format(db=TEST_DB)
-            mocked_init_url.return_value = 'sqlite:///%s' % TEST_DB
+        self.projector = ProjectorDB()
-            self.projector = ProjectorDB()
 
     def tearDown(self):
         """
@@ -192,3 +192,17 @@ class TestProjectorDB(TestCase):
         # THEN: Projector should have the same source entry
         item = self.projector.get_projector_by_id(item_id)
         self.assertTrue(compare_source(item.source_list[0], source))
+
+    def manufacturer_repr_test(self):
+        """
+        Test manufacturer class __repr__ text
+        """
+        # GIVEN: Test object
+        manufacturer = Manufacturer()
+
+        # WHEN: Name is set
+        manufacturer.name = 'OpenLP Test'
+
+        # THEN: __repr__ should return a proper string
+        self.assertEqual(str(manufacturer), '<Manufacturer(name="OpenLP Test")>',
+                         'Manufacturer.__repr__() should have returned a proper representation string')

tests/functional/openlp_plugins/songs/test_mediaitem.py

@@ -23,6 +23,7 @@
 This module contains tests for the lib submodule of the Songs plugin.
 """
 from unittest import TestCase
+from unittest.mock import call
 
 from PyQt5 import QtCore
 
@@ -151,29 +152,7 @@ class TestMediaItem(TestCase, TestMixin):
         # GIVEN: Search results grouped by book and entry, plus a mocked QtListWidgetItem
         with patch('openlp.core.lib.QtWidgets.QListWidgetItem') as MockedQListWidgetItem, \
                 patch('openlp.core.lib.QtCore.Qt.UserRole') as MockedUserRole:
-            mock_search_results = []
+            mock_search_results = [('1', 'My Book', 'My Song', 1)]
-            mock_songbook_entry = MagicMock()
-            mock_songbook_entry_temp = MagicMock()
-            mock_songbook = MagicMock()
-            mock_song = MagicMock()
-            mock_song_temp = MagicMock()
-            mock_songbook_entry.entry = '1'
-            mock_songbook_entry_temp.entry = '2'
-            mock_songbook.name = 'My Book'
-            mock_song.id = 1
-            mock_song.title = 'My Song'
-            mock_song.sort_key = 'My Song'
-            mock_song.temporary = False
-            mock_song_temp.id = 2
-            mock_song_temp.title = 'My Temporary'
-            mock_song_temp.sort_key = 'My Temporary'
-            mock_song_temp.temporary = True
-            mock_songbook_entry.song = mock_song
-            mock_songbook_entry.songbook = mock_songbook
-            mock_songbook_entry_temp.song = mock_song_temp
-            mock_songbook_entry_temp.songbook = mock_songbook
-            mock_search_results.append(mock_songbook_entry)
-            mock_search_results.append(mock_songbook_entry_temp)
             mock_qlist_widget = MagicMock()
             MockedQListWidgetItem.return_value = mock_qlist_widget
 
@@ -183,9 +162,35 @@ class TestMediaItem(TestCase, TestMixin):
             # THEN: The current list view is cleared, the widget is created, and the relevant attributes set
             self.media_item.list_view.clear.assert_called_with()
             MockedQListWidgetItem.assert_called_once_with('My Book #1: My Song')
-            mock_qlist_widget.setData.assert_called_once_with(MockedUserRole, mock_songbook_entry.song.id)
+            mock_qlist_widget.setData.assert_called_once_with(MockedUserRole, 1)
             self.media_item.list_view.addItem.assert_called_once_with(mock_qlist_widget)
 
+    def songbook_natural_sorting_test(self):
+        """
+        Test that songbooks are sorted naturally
+        """
+        # GIVEN: Search results grouped by book and entry, plus a mocked QtListWidgetItem
+        with patch('openlp.core.lib.QtWidgets.QListWidgetItem') as MockedQListWidgetItem:
+            mock_search_results = [('2', 'Thy Book', 'Thy Song', 50),
+                                   ('2', 'My Book', 'Your Song', 7),
+                                   ('10', 'My Book', 'Our Song', 12),
+                                   ('1', 'My Book', 'My Song', 1),
+                                   ('2', 'Thy Book', 'A Song', 8)]
+            mock_qlist_widget = MagicMock()
+            MockedQListWidgetItem.return_value = mock_qlist_widget
+
+            # WHEN: I display song search results grouped by book
+            self.media_item.display_results_book(mock_search_results)
+
+            # THEN: The songbooks are inserted in the right (natural) order,
+            #       grouped first by book, then by number, then by song title
+            calls = [call('My Book #1: My Song'), call().setData(QtCore.Qt.UserRole, 1),
+                     call('My Book #2: Your Song'), call().setData(QtCore.Qt.UserRole, 7),
+                     call('My Book #10: Our Song'), call().setData(QtCore.Qt.UserRole, 12),
+                     call('Thy Book #2: A Song'), call().setData(QtCore.Qt.UserRole, 8),
+                     call('Thy Book #2: Thy Song'), call().setData(QtCore.Qt.UserRole, 50)]
+            MockedQListWidgetItem.assert_has_calls(calls)
+
     def display_results_topic_test(self):
         """
         Test displaying song search results grouped by topic with basic song

tests/functional/openlp_plugins/songs/test_openlpimporter.py

@@ -0,0 +1,75 @@
+# -*- coding: utf-8 -*-
+# vim: autoindent shiftwidth=4 expandtab textwidth=120 tabstop=4 softtabstop=4
+
+###############################################################################
+# OpenLP - Open Source Lyrics Projection                                      #
+# --------------------------------------------------------------------------- #
+# Copyright (c) 2008-2016 OpenLP Developers                                   #
+# --------------------------------------------------------------------------- #
+# This program is free software; you can redistribute it and/or modify it     #
+# under the terms of the GNU General Public License as published by the Free  #
+# Software Foundation; version 2 of the License.                              #
+#                                                                             #
+# This program is distributed in the hope that it will be useful, but WITHOUT #
+# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or       #
+# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for    #
+# more details.                                                               #
+#                                                                             #
+# You should have received a copy of the GNU General Public License along     #
+# with this program; if not, write to the Free Software Foundation, Inc., 59  #
+# Temple Place, Suite 330, Boston, MA 02111-1307 USA                          #
+###############################################################################
+"""
+This module contains tests for the OpenLP song importer.
+"""
+from unittest import TestCase
+
+from openlp.plugins.songs.lib.importers.openlp import OpenLPSongImport
+from openlp.core.common import Registry
+from tests.functional import patch, MagicMock
+
+
+class TestOpenLPImport(TestCase):
+    """
+    Test the functions in the :mod:`openlp` importer module.
+    """
+    def setUp(self):
+        """
+        Create the registry
+        """
+        Registry.create()
+
+    def create_importer_test(self):
+        """
+        Test creating an instance of the OpenLP database importer
+        """
+        # GIVEN: A mocked out SongImport class, and a mocked out "manager"
+        with patch('openlp.plugins.songs.lib.importers.openlp.SongImport'):
+            mocked_manager = MagicMock()
+
+            # WHEN: An importer object is created
+            importer = OpenLPSongImport(mocked_manager, filenames=[])
+
+            # THEN: The importer object should not be None
+            self.assertIsNotNone(importer, 'Import should not be none')
+
+    def invalid_import_source_test(self):
+        """
+        Test OpenLPSongImport.do_import handles different invalid import_source values
+        """
+        # GIVEN: A mocked out SongImport class, and a mocked out "manager"
+        with patch('openlp.plugins.songs.lib.importers.openlp.SongImport'):
+            mocked_manager = MagicMock()
+            mocked_import_wizard = MagicMock()
+            importer = OpenLPSongImport(mocked_manager, filenames=[])
+            importer.import_wizard = mocked_import_wizard
+            importer.stop_import_flag = True
+
+            # WHEN: Import source is not a list
+            for source in ['not a list', 0]:
+                importer.import_source = source
+
+                # THEN: do_import should return none and the progress bar maximum should not be set.
+                self.assertIsNone(importer.do_import(), 'do_import should return None when import_source is not a list')
+                self.assertEqual(mocked_import_wizard.progress_bar.setMaximum.called, False,
+                                 'setMaximum on import_wizard.progress_bar should not have been called')