diff --git a/openlp/plugins/songs/lib/mediaitem.py b/openlp/plugins/songs/lib/mediaitem.py index 8edac2877..11deeb31d 100644 --- a/openlp/plugins/songs/lib/mediaitem.py +++ b/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: - # Do not display temporary songs - if songbook_entry.song.temporary: - continue - song_detail = '%s #%s: %s' % (songbook_entry.songbook.name, songbook_entry.entry, songbook_entry.song.title) + for result in search_results: + song_detail = '%s #%s: %s' % (result[1], result[0], result[2]) 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): diff --git a/tests/functional/openlp_plugins/songs/test_mediaitem.py b/tests/functional/openlp_plugins/songs/test_mediaitem.py index 4b9fd50ee..12447368b 100644 --- a/tests/functional/openlp_plugins/songs/test_mediaitem.py +++ b/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_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_search_results = [('1', 'My Book', 'My Song', 1)] 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 diff --git a/tests/functional/openlp_plugins/songs/test_openlpimporter.py b/tests/functional/openlp_plugins/songs/test_openlpimporter.py index 113db16e0..b78d5c43b 100644 --- a/tests/functional/openlp_plugins/songs/test_openlpimporter.py +++ b/tests/functional/openlp_plugins/songs/test_openlpimporter.py @@ -73,4 +73,3 @@ class TestOpenLPImport(TestCase): 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') -