Code review comment for lp:~samothjtm/openlp/bug-1695587

Here, I've fixed your issues for you and written a test for you. Next time it's on you.

=== modified file 'openlp/plugins/songs/lib/mediaitem.py'
--- openlp/plugins/songs/lib/mediaitem.py 2017-06-03 10:44:28 +0000
+++ openlp/plugins/songs/lib/mediaitem.py 2017-06-04 20:18:23 +0000
@@ -231,13 +231,13 @@

     def search_entire(self, search_keywords):
         search_string = '%{text}%'.format(text=clean_string(search_keywords))
- 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),
- Song.search_title.like(search_string), Song.search_lyrics.like(search_string),
- Song.comments.like(search_string), Song.alternate_title.like(search_string)))
- .all())
+ 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),
+ 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-04 20:21:52 +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,36 @@

         # 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, some keywords and some mocks
+ keywords = '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 = lambda a: a
+ MockedBook.name.like = lambda a: a
+ self.mocked_plugin.manager.get_all_objects.return_value = [mocked_song]
+
+ # WHEN: search_entire_song() is called with the keywords
+ result = self.media_item.search_entire(keywords)
+
+ # 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.comment.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.get_all_objects.assert_called_once_with(MockedSong,
+ 'Jesus Jesus Jesus Jesus Jesus')
+ assert result == [mocked_song]