From bb0adc6f5dc1afdb3a15e69ea8682f8f63675423 Mon Sep 17 00:00:00 2001
From: Chris Hill <chris@minkus.me.uk>
Date: Sat, 19 Mar 2016 15:01:10 +0000
Subject: [PATCH] fixed bug #1280295 'Enable natural sorting for song book
 searches', refactored to move filtering to database, updated test

Fixes: https://launchpad.net/bugs/1280295
---
 openlp/plugins/songs/lib/mediaitem.py         | 30 +++++++----------
 .../openlp_plugins/songs/test_mediaitem.py    | 32 +++++++++++++++++++
 2 files changed, 44 insertions(+), 18 deletions(-)

diff --git a/openlp/plugins/songs/lib/mediaitem.py b/openlp/plugins/songs/lib/mediaitem.py
index 12579c56d..12ef3de3d 100644
--- a/openlp/plugins/songs/lib/mediaitem.py
+++ b/openlp/plugins/songs/lib/mediaitem.py
@@ -203,7 +203,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 + '%'
@@ -288,31 +294,19 @@ 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))
-        songbook_entries = sorted(songbook_entries, key=lambda songbook_entry: (songbook_entry.songbook.name, self._natural_sort_key(songbook_entry.entry)))
-        for songbook_entry in songbook_entries:
+        search_results = sorted(search_results, key=lambda songbook_entry: (songbook_entry.songbook.name, self._natural_sort_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)
diff --git a/tests/functional/openlp_plugins/songs/test_mediaitem.py b/tests/functional/openlp_plugins/songs/test_mediaitem.py
index d09f5b76e..865b81ba9 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