forked from openlp/openlp
Fixes bug #1280295 - 'Enable natural sorting for song book searches' using get_natural_key
Now also includes natural sorting for author, topic, theme & CCLI number as well Also refactors Songbook Search to make the database do filtering for performance rather than querying all then filtering I've tested it on my database of 500-odd songs and it seems a little faster Includes unit tests -------------------------------- lp:~minkus/openlp/naturalsortsongs (revision 2519) [SUCCESS] https://ci.op... bzr-revno: 2637 Fixes: https://launchpad.net/bugs/1280295
This commit is contained in:
chris@minkus.me.uk
Tim Bentley
commit
5a3e5c655c
@ -32,6 +32,7 @@ from openlp.core.common import Registry, AppLocation, Settings, check_directory_
|
||||
from openlp.core.lib import MediaManagerItem, ItemCapabilities, PluginStatus, ServiceItemContext, \
|
||||
check_item_selected, create_separated_list
|
||||
from openlp.core.lib.ui import create_widget_action
|
||||
from openlp.core.utils import get_natural_key
|
||||
from openlp.plugins.songs.forms.editsongform import EditSongForm
|
||||
from openlp.plugins.songs.forms.songmaintenanceform import SongMaintenanceForm
|
||||
from openlp.plugins.songs.forms.songimportform import SongImportForm
|
||||
@ -203,7 +204,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 + '%'
|
||||
@ -278,8 +285,10 @@ class SongMediaItem(MediaManagerItem):
|
||||
"""
|
||||
log.debug('display results Author')
|
||||
self.list_view.clear()
|
||||
search_results = sorted(search_results, key=lambda author: get_natural_key(author.display_name))
|
||||
for author in search_results:
|
||||
for song in author.songs:
|
||||
songs = sorted(author.songs, key=lambda song: song.sort_key)
|
||||
for song in songs:
|
||||
# Do not display temporary songs
|
||||
if song.temporary:
|
||||
continue
|
||||
@ -288,32 +297,20 @@ 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)
|
||||
.order_by(Book.name)
|
||||
.order_by(SongBookEntry.entry))
|
||||
for songbook_entry in songbook_entries:
|
||||
search_results = sorted(search_results, key=lambda songbook_entry:
|
||||
(get_natural_key(songbook_entry.songbook.name), get_natural_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)
|
||||
@ -328,7 +325,7 @@ class SongMediaItem(MediaManagerItem):
|
||||
"""
|
||||
log.debug('display results Topic')
|
||||
self.list_view.clear()
|
||||
search_results = sorted(search_results, key=lambda topic: self._natural_sort_key(topic.name))
|
||||
search_results = sorted(search_results, key=lambda topic: get_natural_key(topic.name))
|
||||
for topic in search_results:
|
||||
songs = sorted(topic.songs, key=lambda song: song.sort_key)
|
||||
for song in songs:
|
||||
@ -349,6 +346,8 @@ class SongMediaItem(MediaManagerItem):
|
||||
"""
|
||||
log.debug('display results Themes')
|
||||
self.list_view.clear()
|
||||
search_results = sorted(search_results, key=lambda song: (get_natural_key(song.theme_name),
|
||||
song.sort_key))
|
||||
for song in search_results:
|
||||
# Do not display temporary songs
|
||||
if song.temporary:
|
||||
@ -367,7 +366,8 @@ class SongMediaItem(MediaManagerItem):
|
||||
"""
|
||||
log.debug('display results CCLI number')
|
||||
self.list_view.clear()
|
||||
songs = sorted(search_results, key=lambda song: self._natural_sort_key(song.ccli_number))
|
||||
songs = sorted(search_results, key=lambda song: (get_natural_key(song.ccli_number),
|
||||
song.sort_key))
|
||||
for song in songs:
|
||||
# Do not display temporary songs
|
||||
if song.temporary:
|
||||
@ -694,14 +694,6 @@ class SongMediaItem(MediaManagerItem):
|
||||
# List must be empty at the end
|
||||
return not author_list
|
||||
|
||||
def _natural_sort_key(self, s):
|
||||
"""
|
||||
Return a tuple by which s is sorted.
|
||||
:param s: A string value from the list we want to sort.
|
||||
"""
|
||||
return [int(text) if text.isdecimal() else text.lower()
|
||||
for text in re.split('(\d+)', s)]
|
||||
|
||||
def search(self, string, show_error):
|
||||
"""
|
||||
Search for some songs
|
||||
|
||||
@ -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
|
||||
@ -416,19 +448,6 @@ class TestMediaItem(TestCase, TestMixin):
|
||||
# THEN: They should not match
|
||||
self.assertFalse(result, "Authors should not match")
|
||||
|
||||
def natural_sort_key_test(self):
|
||||
"""
|
||||
Test the _natural_sort_key function
|
||||
"""
|
||||
# GIVEN: A string to be converted into a sort key
|
||||
string_sort_key = 'A1B12C'
|
||||
|
||||
# WHEN: We attempt to create a sort key
|
||||
sort_key_result = self.media_item._natural_sort_key(string_sort_key)
|
||||
|
||||
# THEN: We should get back a tuple split on integers
|
||||
self.assertEqual(sort_key_result, ['a', 1, 'b', 12, 'c'])
|
||||
|
||||
def build_remote_search_test(self):
|
||||
"""
|
||||
Test results for the remote search api
|
||||
|
||||
Loading…
Reference in New Issue
Block a user