From 0c8f9e3b712b50b07901c29972e28f2ca43c1f58 Mon Sep 17 00:00:00 2001 From: Chris Hill Date: Sun, 17 Apr 2016 22:22:30 +0100 Subject: [PATCH] Removed auto-select for searches where multiple IDs selected, added tests to hide temporary songs --- openlp/plugins/songs/lib/mediaitem.py | 26 +----- .../openlp_plugins/songs/test_mediaitem.py | 83 +++++++++++++++---- 2 files changed, 66 insertions(+), 43 deletions(-) diff --git a/openlp/plugins/songs/lib/mediaitem.py b/openlp/plugins/songs/lib/mediaitem.py index 0bbea1fa0..2c483a48a 100644 --- a/openlp/plugins/songs/lib/mediaitem.py +++ b/openlp/plugins/songs/lib/mediaitem.py @@ -284,7 +284,6 @@ class SongMediaItem(MediaManagerItem): :return: None """ log.debug('display results Author') - self.save_auto_select_id() self.list_view.clear() search_results.sort(key=lambda author: get_natural_key(author.display_name)) for author in search_results: @@ -297,10 +296,6 @@ class SongMediaItem(MediaManagerItem): song_name = QtWidgets.QListWidgetItem(song_detail) song_name.setData(QtCore.Qt.UserRole, song.id) self.list_view.addItem(song_name) - # Auto-select the item if name has been set - if song.id == self.auto_select_id: - self.list_view.setCurrentItem(song_name) - self.auto_select_id = -1 def display_results_book(self, search_results): """ @@ -310,21 +305,17 @@ class SongMediaItem(MediaManagerItem): :return: None """ log.debug('display results Book') - self.save_auto_select_id() self.list_view.clear() search_results.sort(key=lambda songbook_entry: (get_natural_key(songbook_entry.songbook.name), get_natural_key(songbook_entry.entry))) 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) song_name = QtWidgets.QListWidgetItem(song_detail) song_name.setData(QtCore.Qt.UserRole, songbook_entry.song.id) self.list_view.addItem(song_name) - # Auto-select the item if name has been set - if songbook_entry.song.id == self.auto_select_id: - self.list_view.setCurrentItem(song_name) - self.auto_select_id = -1 def display_results_topic(self, search_results): """ @@ -334,7 +325,6 @@ class SongMediaItem(MediaManagerItem): :return: None """ log.debug('display results Topic') - self.save_auto_select_id() self.list_view.clear() search_results.sort(key=lambda topic: get_natural_key(topic.name)) for topic in search_results: @@ -347,10 +337,6 @@ class SongMediaItem(MediaManagerItem): song_name = QtWidgets.QListWidgetItem(song_detail) song_name.setData(QtCore.Qt.UserRole, song.id) self.list_view.addItem(song_name) - # Auto-select the item if name has been set - if song.id == self.auto_select_id: - self.list_view.setCurrentItem(song_name) - self.auto_select_id = -1 def display_results_themes(self, search_results): """ @@ -360,7 +346,6 @@ class SongMediaItem(MediaManagerItem): :return: None """ log.debug('display results Themes') - self.save_auto_select_id() self.list_view.clear() search_results.sort(key=lambda song: (get_natural_key(song.theme_name), song.sort_key)) @@ -372,10 +357,6 @@ class SongMediaItem(MediaManagerItem): song_name = QtWidgets.QListWidgetItem(song_detail) song_name.setData(QtCore.Qt.UserRole, song.id) self.list_view.addItem(song_name) - # Auto-select the item if name has been set - if song.id == self.auto_select_id: - self.list_view.setCurrentItem(song_name) - self.auto_select_id = -1 def display_results_cclinumber(self, search_results): """ @@ -385,7 +366,6 @@ class SongMediaItem(MediaManagerItem): :return: None """ log.debug('display results CCLI number') - self.save_auto_select_id() self.list_view.clear() search_results.sort(key=lambda song: (get_natural_key(song.ccli_number), song.sort_key)) @@ -397,10 +377,6 @@ class SongMediaItem(MediaManagerItem): song_name = QtWidgets.QListWidgetItem(song_detail) song_name.setData(QtCore.Qt.UserRole, song.id) self.list_view.addItem(song_name) - # Auto-select the item if name has been set - if song.id == self.auto_select_id: - self.list_view.setCurrentItem(song_name) - self.auto_select_id = -1 def on_clear_text_button_click(self): """ 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): """