I've got a couple of comments below for you to take a look at.
Also, as previously noted, you need a code test before we'll accept your merge
proposal. I'm afraid that manual testing, while nice, does not constitute a
test for merging purposes.
Diff comments:
> === 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-04 14:21:07 +0000
> @@ -231,9 +231,13 @@
>
> 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)
Why the extra set of brackets? Use \ for line continuation.
> + .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)))
search_title already contains a searchable form of alternate_title.
> + .all())
>
> def on_song_list_load(self):
> """
--
https://code.launchpad.net/~samothjtm/openlp/bug-1695587/+merge/325035
Your team OpenLP Core is subscribed to branch lp:openlp.
_______________________________________________
Mailing list: https://launchpad.net/~openlp-core
Post to : [email protected]
Unsubscribe : https://launchpad.net/~openlp-core
More help : https://help.launchpad.net/ListHelp
