openlp-core team mailing list archive

[Johannes Thomas Meyer <johannes@xxxxxxxxxx>]

Johannes Thomas Meyer has proposed merging lp:~samothjtm/openlp/bug-1695587 into lp:openlp.

Commit message:
added SongBook name and Song Number to "Entire Song" Search

Requested reviews:
  Tomas Groth (tomasgroth)
  Raoul Snyman (raoul-snyman)
  Samuel Mehrbrodt (sam92)
Related bugs:
  Bug #1695587 in OpenLP: "Entire Song doesn't search Song Number"
  https://bugs.launchpad.net/openlp/+bug/1695587

For more details, see:
https://code.launchpad.net/~samothjtm/openlp/bug-1695587/+merge/325118

added SongBook name and Song Number to "Entire Song" Search

lp:~samothjtm/openlp/bug-1695587 (revision 2748)
[SUCCESS] https://ci.openlp.io/job/Branch-01-Pull/2076/
[SUCCESS] https://ci.openlp.io/job/Branch-02-Functional-Tests/1986/
[SUCCESS] https://ci.openlp.io/job/Branch-03-Interface-Tests/1902/
[SUCCESS] https://ci.openlp.io/job/Branch-04a-Code_Analysis/1280/
[SUCCESS] https://ci.openlp.io/job/Branch-04b-Test_Coverage/1130/
[SUCCESS] https://ci.openlp.io/job/Branch-04c-Code_Analysis2/259/
[SUCCESS] https://ci.openlp.io/job/Branch-05-AppVeyor-Tests/105/

-- 
Your team OpenLP Core is subscribed to branch lp:openlp.

=== modified file 'openlp/plugins/songs/lib/mediaitem.py'
--- openlp/plugins/songs/lib/mediaitem.py	2017-01-25 21:17:27 +0000
+++ openlp/plugins/songs/lib/mediaitem.py	2017-06-05 21:58:05 +0000
@@ -231,9 +231,14 @@
 
     def search_entire(self, search_keywords):
         search_string = '%{text}%'.format(text=clean_string(search_keywords))
-        return self.plugin.manager.get_all_objects(
-            Song, or_(Song.search_title.like(search_string), Song.search_lyrics.like(search_string),
-                      Song.comments.like(search_string)))
+        return self.plugin.manager.session.query(Song) \
+            .join(SongBookEntry, isouter=True) \
+            .join(Book, isouter=True) \
+            .filter(or_(Book.name.like(search_string), SongBookEntry.entry.like(search_string),
+                        # hint: search_title contains alternate title
+                        Song.search_title.like(search_string), Song.search_lyrics.like(search_string),
+                        Song.comments.like(search_string))) \
+            .all()
 
     def on_song_list_load(self):
         """

=== modified file 'tests/functional/openlp_plugins/songs/test_mediaitem.py'
--- tests/functional/openlp_plugins/songs/test_mediaitem.py	2017-04-24 05:17:55 +0000
+++ tests/functional/openlp_plugins/songs/test_mediaitem.py	2017-06-05 21:58:05 +0000
@@ -46,9 +46,10 @@
         Registry.create()
         Registry().register('service_list', MagicMock())
         Registry().register('main_window', MagicMock())
+        self.mocked_plugin = MagicMock()
         with patch('openlp.core.lib.mediamanageritem.MediaManagerItem._setup'), \
                 patch('openlp.plugins.songs.forms.editsongform.EditSongForm.__init__'):
-            self.media_item = SongMediaItem(None, MagicMock())
+            self.media_item = SongMediaItem(None, self.mocked_plugin)
             self.media_item.save_auto_select_id = MagicMock()
             self.media_item.list_view = MagicMock()
             self.media_item.list_view.save_auto_select_id = MagicMock()
@@ -558,3 +559,35 @@
 
         # THEN: The correct formatted results are returned
         self.assertEqual(search_results, [[123, 'My Song', 'My alternative']])
+
+    @patch('openlp.plugins.songs.lib.mediaitem.Book')
+    @patch('openlp.plugins.songs.lib.mediaitem.SongBookEntry')
+    @patch('openlp.plugins.songs.lib.mediaitem.Song')
+    @patch('openlp.plugins.songs.lib.mediaitem.or_')
+    def test_entire_song_search(self, mocked_or, MockedSong, MockedSongBookEntry, MockedBook):
+        """
+        Test that searching the entire song does the right queries
+        """
+        # GIVEN: A song media item, a keyword and some mocks
+        keyword = 'Jesus'
+        mocked_song = MagicMock()
+        mocked_or.side_effect = lambda a, b, c, d, e: ' '.join([a, b, c, d, e])
+        MockedSong.search_title.like.side_effect = lambda a: a
+        MockedSong.search_lyrics.like.side_effect = lambda a: a
+        MockedSong.comments.like.side_effect = lambda a: a
+        MockedSongBookEntry.entry.like.side_effect = lambda a: a
+        MockedBook.name.like.side_effect = lambda a: a
+
+        # WHEN: search_entire_song() is called with the keyword
+        self.media_item.search_entire(keyword)
+
+        # THEN: The correct calls were made
+        MockedSong.search_title.like.assert_called_once_with('%jesus%')
+        MockedSong.search_lyrics.like.assert_called_once_with('%jesus%')
+        MockedSong.comments.like.assert_called_once_with('%jesus%')
+        MockedSongBookEntry.entry.like.assert_called_once_with('%jesus%')
+        MockedBook.name.like.assert_called_once_with('%jesus%')
+        mocked_or.assert_called_once_with('%jesus%', '%jesus%', '%jesus%', '%jesus%', '%jesus%')
+        self.mocked_plugin.manager.session.query.assert_called_once_with(MockedSong)
+
+        self.assertEqual(self.mocked_plugin.manager.session.query.mock_calls[4][0], '().join().join().filter().all')