diff --git a/openlp/plugins/songs/lib/mediaitem.py b/openlp/plugins/songs/lib/mediaitem.py index 8772f0771..b84e557c2 100644 --- a/openlp/plugins/songs/lib/mediaitem.py +++ b/openlp/plugins/songs/lib/mediaitem.py @@ -32,6 +32,7 @@ from openlp.core.common import Registry, AppLocation, Settings, check_directory_ from openlp.core.lib import MediaManagerItem, ItemCapabilities, PluginStatus, ServiceItemContext, \ check_item_selected, create_separated_list from openlp.core.lib.ui import create_widget_action +from openlp.core.utils import get_natural_key from openlp.plugins.songs.forms.editsongform import EditSongForm from openlp.plugins.songs.forms.songmaintenanceform import SongMaintenanceForm from openlp.plugins.songs.forms.songimportform import SongImportForm @@ -203,7 +204,13 @@ class SongMediaItem(MediaManagerItem): self.display_results_topic(search_results) elif search_type == SongSearch.Books: log.debug('Songbook Search') - self.display_results_book(search_keywords) + search_keywords = search_keywords.rpartition(' ') + search_book = search_keywords[0] + '%' + search_entry = search_keywords[2] + '%' + search_results = (self.plugin.manager.session.query(SongBookEntry) + .join(Book) + .filter(Book.name.like(search_book), SongBookEntry.entry.like(search_entry)).all()) + self.display_results_book(search_results) elif search_type == SongSearch.Themes: log.debug('Theme Search') search_string = '%' + search_keywords + '%' @@ -278,8 +285,10 @@ class SongMediaItem(MediaManagerItem): """ log.debug('display results Author') self.list_view.clear() + search_results = sorted(search_results, key=lambda author: get_natural_key(author.display_name)) for author in search_results: - for song in author.songs: + songs = sorted(author.songs, key=lambda song: song.sort_key) + for song in songs: # Do not display temporary songs if song.temporary: continue @@ -288,32 +297,20 @@ class SongMediaItem(MediaManagerItem): song_name.setData(QtCore.Qt.UserRole, song.id) self.list_view.addItem(song_name) - def display_results_book(self, search_keywords): + def display_results_book(self, search_results): """ - Display the song search results in the media manager list, grouped by book + Display the song search results in the media manager list, grouped by book and entry - :param search_keywords: A list of search keywords - book first, then number + :param search_results: A list of db SongBookEntry objects :return: None """ - log.debug('display results Book') self.list_view.clear() - - search_keywords = search_keywords.rpartition(' ') - search_book = search_keywords[0] - search_entry = re.sub(r'[^0-9]', '', search_keywords[2]) - - songbook_entries = (self.plugin.manager.session.query(SongBookEntry) - .join(Book) - .order_by(Book.name) - .order_by(SongBookEntry.entry)) - for songbook_entry in songbook_entries: + search_results = sorted(search_results, key=lambda songbook_entry: + (get_natural_key(songbook_entry.songbook.name), get_natural_key(songbook_entry.entry))) + for songbook_entry in search_results: if songbook_entry.song.temporary: continue - if search_book.lower() not in songbook_entry.songbook.name.lower(): - continue - if search_entry not in songbook_entry.entry: - 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) @@ -328,7 +325,7 @@ class SongMediaItem(MediaManagerItem): """ log.debug('display results Topic') self.list_view.clear() - search_results = sorted(search_results, key=lambda topic: self._natural_sort_key(topic.name)) + search_results = sorted(search_results, key=lambda topic: get_natural_key(topic.name)) for topic in search_results: songs = sorted(topic.songs, key=lambda song: song.sort_key) for song in songs: @@ -349,6 +346,8 @@ class SongMediaItem(MediaManagerItem): """ log.debug('display results Themes') self.list_view.clear() + search_results = sorted(search_results, key=lambda song: (get_natural_key(song.theme_name), + song.sort_key)) for song in search_results: # Do not display temporary songs if song.temporary: @@ -367,7 +366,8 @@ class SongMediaItem(MediaManagerItem): """ log.debug('display results CCLI number') self.list_view.clear() - songs = sorted(search_results, key=lambda song: self._natural_sort_key(song.ccli_number)) + songs = sorted(search_results, key=lambda song: (get_natural_key(song.ccli_number), + song.sort_key)) for song in songs: # Do not display temporary songs if song.temporary: @@ -694,14 +694,6 @@ class SongMediaItem(MediaManagerItem): # List must be empty at the end return not author_list - def _natural_sort_key(self, s): - """ - Return a tuple by which s is sorted. - :param s: A string value from the list we want to sort. - """ - return [int(text) if text.isdecimal() else text.lower() - for text in re.split('(\d+)', s)] - def search(self, string, show_error): """ Search for some songs diff --git a/tests/functional/openlp_plugins/songs/test_mediaitem.py b/tests/functional/openlp_plugins/songs/test_mediaitem.py index 4ae15909b..3cd5f97ba 100644 --- a/tests/functional/openlp_plugins/songs/test_mediaitem.py +++ b/tests/functional/openlp_plugins/songs/test_mediaitem.py @@ -127,6 +127,38 @@ class TestMediaItem(TestCase, TestMixin): mock_qlist_widget.setData.assert_called_with(MockedUserRole, mock_song.id) self.media_item.list_view.addItem.assert_called_with(mock_qlist_widget) + def display_results_book_test(self): + """ + Test displaying song search results grouped by book and entry with basic song + """ + # 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 = MagicMock() + mock_song = MagicMock() + mock_songbook_entry.entry = '1' + 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_songbook_entry.song = mock_song + mock_songbook_entry.songbook = mock_songbook + mock_search_results.append(mock_songbook_entry) + 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 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_with('My Book #1: My Song') + mock_qlist_widget.setData.assert_called_with(MockedUserRole, mock_songbook_entry.song.id) + self.media_item.list_view.addItem.assert_called_with(mock_qlist_widget) + def display_results_topic_test(self): """ Test displaying song search results grouped by topic with basic song @@ -416,19 +448,6 @@ class TestMediaItem(TestCase, TestMixin): # THEN: They should not match self.assertFalse(result, "Authors should not match") - def natural_sort_key_test(self): - """ - Test the _natural_sort_key function - """ - # GIVEN: A string to be converted into a sort key - string_sort_key = 'A1B12C' - - # WHEN: We attempt to create a sort key - sort_key_result = self.media_item._natural_sort_key(string_sort_key) - - # THEN: We should get back a tuple split on integers - self.assertEqual(sort_key_result, ['a', 1, 'b', 12, 'c']) - def build_remote_search_test(self): """ Test results for the remote search api