diff --git a/openlp/plugins/songs/forms/editsongform.py b/openlp/plugins/songs/forms/editsongform.py index 678169e64..b33788a4c 100644 --- a/openlp/plugins/songs/forms/editsongform.py +++ b/openlp/plugins/songs/forms/editsongform.py @@ -34,6 +34,7 @@ from PyQt5 import QtCore, QtWidgets from openlp.core.common import Registry, RegistryProperties, AppLocation, UiStrings, check_directory_exists, translate from openlp.core.lib import FileDialog, PluginStatus, MediaType, create_separated_list from openlp.core.lib.ui import set_case_insensitive_completer, critical_error_message_box, find_and_set_in_combo_box +from openlp.core.common.languagemanager import get_natural_key from openlp.plugins.songs.lib import VerseType, clean_song from openlp.plugins.songs.lib.db import Book, Song, Author, AuthorType, Topic, MediaFile, SongBookEntry from openlp.plugins.songs.lib.ui import SongStrings @@ -110,7 +111,12 @@ class EditSongForm(QtWidgets.QDialog, Ui_EditSongDialog, RegistryProperties): """ Generically load a set of objects into a cache and a combobox. """ - objects = self.manager.get_all_objects(cls, order_by_ref=cls.name) + def get_key(obj): + """Get the key to sort by""" + return get_natural_key(obj.name) + + objects = self.manager.get_all_objects(cls) + objects.sort(key=get_key) combo.clear() combo.addItem('') for obj in objects: @@ -343,7 +349,12 @@ class EditSongForm(QtWidgets.QDialog, Ui_EditSongDialog, RegistryProperties): """ Load the authors from the database into the combobox. """ - authors = self.manager.get_all_objects(Author, order_by_ref=Author.display_name) + def get_author_key(author): + """Get the key to sort by""" + return get_natural_key(author.display_name) + + authors = self.manager.get_all_objects(Author) + authors.sort(key=get_author_key) self.authors_combo_box.clear() self.authors_combo_box.addItem('') self.authors = [] @@ -378,9 +389,14 @@ class EditSongForm(QtWidgets.QDialog, Ui_EditSongDialog, RegistryProperties): """ Load the themes into a combobox. """ + def get_theme_key(theme): + """Get the key to sort by""" + return get_natural_key(theme) + self.theme_combo_box.clear() self.theme_combo_box.addItem('') self.themes = theme_list + self.themes.sort(key=get_theme_key) self.theme_combo_box.addItems(theme_list) set_case_insensitive_completer(self.themes, self.theme_combo_box) diff --git a/openlp/plugins/songs/forms/songexportform.py b/openlp/plugins/songs/forms/songexportform.py index ba8e2738a..e8a559c44 100644 --- a/openlp/plugins/songs/forms/songexportform.py +++ b/openlp/plugins/songs/forms/songexportform.py @@ -203,6 +203,10 @@ class SongExportForm(OpenLPWizard): """ Set default form values for the song export wizard. """ + def get_song_key(song): + """Get the key to sort by""" + return song.sort_key + self.restart() self.finish_button.setVisible(False) self.cancel_button.setVisible(True) @@ -213,7 +217,7 @@ class SongExportForm(OpenLPWizard): # Load the list of songs. self.application.set_busy_cursor() songs = self.plugin.manager.get_all_objects(Song) - songs.sort(key=lambda song: song.sort_key) + songs.sort(key=get_song_key) for song in songs: # No need to export temporary songs. if song.temporary: diff --git a/openlp/plugins/songs/forms/songmaintenanceform.py b/openlp/plugins/songs/forms/songmaintenanceform.py index 1fdfb74d4..74462e6d0 100644 --- a/openlp/plugins/songs/forms/songmaintenanceform.py +++ b/openlp/plugins/songs/forms/songmaintenanceform.py @@ -27,6 +27,7 @@ from sqlalchemy.sql import and_ from openlp.core.common import Registry, RegistryProperties, UiStrings, translate from openlp.core.lib.ui import critical_error_message_box +from openlp.core.common.languagemanager import get_natural_key from openlp.plugins.songs.forms.authorsform import AuthorsForm from openlp.plugins.songs.forms.topicsform import TopicsForm from openlp.plugins.songs.forms.songbookform import SongBookForm @@ -120,8 +121,13 @@ class SongMaintenanceForm(QtWidgets.QDialog, Ui_SongMaintenanceDialog, RegistryP """ Reloads the Authors list. """ + def get_author_key(author): + """Get the key to sort by""" + return get_natural_key(author.display_name) + self.authors_list_widget.clear() - authors = self.manager.get_all_objects(Author, order_by_ref=Author.display_name) + authors = self.manager.get_all_objects(Author) + authors.sort(key=get_author_key) for author in authors: if author.display_name: author_name = QtWidgets.QListWidgetItem(author.display_name) @@ -134,8 +140,13 @@ class SongMaintenanceForm(QtWidgets.QDialog, Ui_SongMaintenanceDialog, RegistryP """ Reloads the Topics list. """ + def get_topic_key(topic): + """Get the key to sort by""" + return get_natural_key(topic.name) + self.topics_list_widget.clear() - topics = self.manager.get_all_objects(Topic, order_by_ref=Topic.name) + topics = self.manager.get_all_objects(Topic) + topics.sort(key=get_topic_key) for topic in topics: topic_name = QtWidgets.QListWidgetItem(topic.name) topic_name.setData(QtCore.Qt.UserRole, topic.id) @@ -145,8 +156,13 @@ class SongMaintenanceForm(QtWidgets.QDialog, Ui_SongMaintenanceDialog, RegistryP """ Reloads the Books list. """ + def get_book_key(book): + """Get the key to sort by""" + return get_natural_key(book.name) + self.song_books_list_widget.clear() - books = self.manager.get_all_objects(Book, order_by_ref=Book.name) + books = self.manager.get_all_objects(Book) + books.sort(key=get_book_key) for book in books: book_name = QtWidgets.QListWidgetItem('%s (%s)' % (book.name, book.publisher)) book_name.setData(QtCore.Qt.UserRole, book.id) diff --git a/openlp/plugins/songs/lib/mediaitem.py b/openlp/plugins/songs/lib/mediaitem.py index d724bfaf2..8edac2877 100644 --- a/openlp/plugins/songs/lib/mediaitem.py +++ b/openlp/plugins/songs/lib/mediaitem.py @@ -194,13 +194,13 @@ class SongMediaItem(MediaManagerItem): log.debug('Authors Search') search_string = '%' + search_keywords + '%' search_results = self.plugin.manager.get_all_objects( - Author, Author.display_name.like(search_string), Author.display_name.asc()) + Author, Author.display_name.like(search_string)) self.display_results_author(search_results) elif search_type == SongSearch.Topics: log.debug('Topics Search') search_string = '%' + search_keywords + '%' search_results = self.plugin.manager.get_all_objects( - Topic, Topic.name.like(search_string), Topic.name.asc()) + Topic, Topic.name.like(search_string)) self.display_results_topic(search_results) elif search_type == SongSearch.Books: log.debug('Songbook Search') @@ -215,7 +215,7 @@ class SongMediaItem(MediaManagerItem): log.debug('Theme Search') search_string = '%' + search_keywords + '%' search_results = self.plugin.manager.get_all_objects( - Song, Song.theme_name.like(search_string), Song.theme_name.asc()) + Song, Song.theme_name.like(search_string)) self.display_results_themes(search_results) elif search_type == SongSearch.Copyright: log.debug('Copyright Search') @@ -258,10 +258,14 @@ class SongMediaItem(MediaManagerItem): :param search_results: A list of db Song objects :return: None """ + def get_song_key(song): + """Get the key to sort by""" + return song.sort_key + log.debug('display results Song') self.save_auto_select_id() self.list_view.clear() - search_results.sort(key=lambda song: song.sort_key) + search_results.sort(key=get_song_key) for song in search_results: # Do not display temporary songs if song.temporary: @@ -283,12 +287,20 @@ class SongMediaItem(MediaManagerItem): :param search_results: A list of db Author objects :return: None """ + def get_author_key(author): + """Get the key to sort by""" + return get_natural_key(author.display_name) + + def get_song_key(song): + """Get the key to sort by""" + return song.sort_key + log.debug('display results Author') self.list_view.clear() - search_results = sorted(search_results, key=lambda author: get_natural_key(author.display_name)) + search_results.sort(key=get_author_key) for author in search_results: - songs = sorted(author.songs, key=lambda song: song.sort_key) - for song in songs: + author.songs.sort(key=get_song_key) + for song in author.songs: # Do not display temporary songs if song.temporary: continue @@ -304,11 +316,15 @@ class SongMediaItem(MediaManagerItem): :param search_results: A list of db SongBookEntry objects :return: None """ + def get_songbook_key(songbook_entry): + """Get the key to sort by""" + return (get_natural_key(songbook_entry.songbook.name), get_natural_key(songbook_entry.entry)) + log.debug('display results Book') self.list_view.clear() - search_results = sorted(search_results, key=lambda songbook_entry: - (get_natural_key(songbook_entry.songbook.name), get_natural_key(songbook_entry.entry))) + 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) @@ -323,12 +339,20 @@ class SongMediaItem(MediaManagerItem): :param search_results: A list of db Topic objects :return: None """ + def get_topic_key(topic): + """Get the key to sort by""" + return get_natural_key(topic.name) + + def get_song_key(song): + """Get the key to sort by""" + return song.sort_key + log.debug('display results Topic') self.list_view.clear() - search_results = sorted(search_results, key=lambda topic: get_natural_key(topic.name)) + search_results.sort(key=get_topic_key) for topic in search_results: - songs = sorted(topic.songs, key=lambda song: song.sort_key) - for song in songs: + topic.songs.sort(key=get_song_key) + for song in topic.songs: # Do not display temporary songs if song.temporary: continue @@ -344,10 +368,13 @@ class SongMediaItem(MediaManagerItem): :param search_results: A list of db Song objects :return: None """ + def get_theme_key(song): + """Get the key to sort by""" + return (get_natural_key(song.theme_name), song.sort_key) + 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)) + search_results.sort(key=get_theme_key) for song in search_results: # Do not display temporary songs if song.temporary: @@ -364,11 +391,14 @@ class SongMediaItem(MediaManagerItem): :param search_results: A list of db Song objects :return: None """ + def get_cclinumber_key(song): + """Get the key to sort by""" + return (get_natural_key(song.ccli_number), song.sort_key) + log.debug('display results CCLI number') self.list_view.clear() - songs = sorted(search_results, key=lambda song: (get_natural_key(song.ccli_number), - song.sort_key)) - for song in songs: + search_results.sort(key=get_cclinumber_key) + for song in search_results: # Do not display temporary songs if song.temporary: continue diff --git a/tests/functional/openlp_plugins/songs/test_mediaitem.py b/tests/functional/openlp_plugins/songs/test_mediaitem.py index 3cd5f97ba..4b9fd50ee 100644 --- a/tests/functional/openlp_plugins/songs/test_mediaitem.py +++ b/tests/functional/openlp_plugins/songs/test_mediaitem.py @@ -53,6 +53,7 @@ class TestMediaItem(TestCase, TestMixin): self.media_item.list_view.save_auto_select_id = MagicMock() self.media_item.list_view.clear = MagicMock() self.media_item.list_view.addItem = MagicMock() + self.media_item.list_view.setCurrentItem = MagicMock() self.media_item.auto_select_id = -1 self.media_item.display_songbook = False self.media_item.display_copyright_symbol = False @@ -79,13 +80,22 @@ class TestMediaItem(TestCase, TestMixin): mock_song.title = 'My Song' mock_song.sort_key = 'My Song' mock_song.authors = [] + mock_song_temp = MagicMock() + mock_song_temp.id = 2 + mock_song_temp.title = 'My Temporary' + mock_song_temp.sort_key = 'My Temporary' + mock_song_temp.authors = [] mock_author = MagicMock() mock_author.display_name = 'My Author' mock_song.authors.append(mock_author) + mock_song_temp.authors.append(mock_author) mock_song.temporary = False + mock_song_temp.temporary = True mock_search_results.append(mock_song) + mock_search_results.append(mock_song_temp) mock_qlist_widget = MagicMock() MockedQListWidgetItem.return_value = mock_qlist_widget + self.media_item.auto_select_id = 1 # WHEN: I display song search results self.media_item.display_results_song(mock_search_results) @@ -93,9 +103,10 @@ 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() self.media_item.save_auto_select_id.assert_called_with() - MockedQListWidgetItem.assert_called_with('My Song (My Author)') - mock_qlist_widget.setData.assert_called_with(MockedUserRole, mock_song.id) - self.media_item.list_view.addItem.assert_called_with(mock_qlist_widget) + MockedQListWidgetItem.assert_called_once_with('My Song (My Author)') + mock_qlist_widget.setData.assert_called_once_with(MockedUserRole, mock_song.id) + self.media_item.list_view.addItem.assert_called_once_with(mock_qlist_widget) + self.media_item.list_view.setCurrentItem.assert_called_with(mock_qlist_widget) def display_results_author_test(self): """ @@ -107,13 +118,19 @@ class TestMediaItem(TestCase, TestMixin): mock_search_results = [] mock_author = MagicMock() mock_song = MagicMock() + mock_song_temp = MagicMock() mock_author.display_name = 'My Author' mock_author.songs = [] 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_author.songs.append(mock_song) + mock_author.songs.append(mock_song_temp) mock_search_results.append(mock_author) mock_qlist_widget = MagicMock() MockedQListWidgetItem.return_value = mock_qlist_widget @@ -123,9 +140,9 @@ 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_with('My Author (My Song)') - mock_qlist_widget.setData.assert_called_with(MockedUserRole, mock_song.id) - self.media_item.list_view.addItem.assert_called_with(mock_qlist_widget) + MockedQListWidgetItem.assert_called_once_with('My Author (My Song)') + mock_qlist_widget.setData.assert_called_once_with(MockedUserRole, mock_song.id) + self.media_item.list_view.addItem.assert_called_once_with(mock_qlist_widget) def display_results_book_test(self): """ @@ -136,17 +153,27 @@ class TestMediaItem(TestCase, TestMixin): 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_qlist_widget = MagicMock() MockedQListWidgetItem.return_value = mock_qlist_widget @@ -155,9 +182,9 @@ 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_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) + MockedQListWidgetItem.assert_called_once_with('My Book #1: My Song') + mock_qlist_widget.setData.assert_called_once_with(MockedUserRole, mock_songbook_entry.song.id) + self.media_item.list_view.addItem.assert_called_once_with(mock_qlist_widget) def display_results_topic_test(self): """ @@ -169,13 +196,19 @@ class TestMediaItem(TestCase, TestMixin): mock_search_results = [] mock_topic = MagicMock() mock_song = MagicMock() + mock_song_temp = MagicMock() mock_topic.name = 'My Topic' mock_topic.songs = [] 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_topic.songs.append(mock_song) + mock_topic.songs.append(mock_song_temp) mock_search_results.append(mock_topic) mock_qlist_widget = MagicMock() MockedQListWidgetItem.return_value = mock_qlist_widget @@ -185,9 +218,9 @@ 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_with('My Topic (My Song)') - mock_qlist_widget.setData.assert_called_with(MockedUserRole, mock_song.id) - self.media_item.list_view.addItem.assert_called_with(mock_qlist_widget) + MockedQListWidgetItem.assert_called_once_with('My Topic (My Song)') + mock_qlist_widget.setData.assert_called_once_with(MockedUserRole, mock_song.id) + self.media_item.list_view.addItem.assert_called_once_with(mock_qlist_widget) def display_results_themes_test(self): """ @@ -198,12 +231,19 @@ class TestMediaItem(TestCase, TestMixin): patch('openlp.core.lib.QtCore.Qt.UserRole') as MockedUserRole: mock_search_results = [] mock_song = MagicMock() + mock_song_temp = MagicMock() mock_song.id = 1 mock_song.title = 'My Song' mock_song.sort_key = 'My Song' mock_song.theme_name = 'My Theme' 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.theme_name = 'My Theme' + mock_song_temp.temporary = True mock_search_results.append(mock_song) + mock_search_results.append(mock_song_temp) mock_qlist_widget = MagicMock() MockedQListWidgetItem.return_value = mock_qlist_widget @@ -212,9 +252,9 @@ 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_with('My Theme (My Song)') - mock_qlist_widget.setData.assert_called_with(MockedUserRole, mock_song.id) - self.media_item.list_view.addItem.assert_called_with(mock_qlist_widget) + MockedQListWidgetItem.assert_called_once_with('My Theme (My Song)') + mock_qlist_widget.setData.assert_called_once_with(MockedUserRole, mock_song.id) + self.media_item.list_view.addItem.assert_called_once_with(mock_qlist_widget) def display_results_cclinumber_test(self): """ @@ -225,12 +265,19 @@ class TestMediaItem(TestCase, TestMixin): patch('openlp.core.lib.QtCore.Qt.UserRole') as MockedUserRole: mock_search_results = [] mock_song = MagicMock() + mock_song_temp = MagicMock() mock_song.id = 1 mock_song.title = 'My Song' mock_song.sort_key = 'My Song' mock_song.ccli_number = '12345' 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.ccli_number = '12346' + mock_song_temp.temporary = True mock_search_results.append(mock_song) + mock_search_results.append(mock_song_temp) mock_qlist_widget = MagicMock() MockedQListWidgetItem.return_value = mock_qlist_widget @@ -239,9 +286,9 @@ 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_with('12345 (My Song)') - mock_qlist_widget.setData.assert_called_with(MockedUserRole, mock_song.id) - self.media_item.list_view.addItem.assert_called_with(mock_qlist_widget) + MockedQListWidgetItem.assert_called_once_with('12345 (My Song)') + mock_qlist_widget.setData.assert_called_once_with(MockedUserRole, mock_song.id) + self.media_item.list_view.addItem.assert_called_once_with(mock_qlist_widget) def build_song_footer_one_author_test(self): """