forked from openlp/openlp
Fix up a potential bug in SongSelect where authors with a single name would crash the importer.
Add this to your merge proposal: -------------------------------- lp:~raoul-snyman/openlp/song-select-fixes (revision 2578) [SUCCESS] https://ci.openlp.io/job/Branch-01-Pull/1199/ [SUCCESS] https://ci.openlp.io/job/Branch-02-Functional-Tests/1124/ [SUCCESS] https://ci.openlp.io/job/Branch-03-Interface-Tests/1063/ [FAILURE] https://ci.openlp.io/job/Branch-04a-Windows_Functional_Tests/909/ Stopping ... bzr-revno: 2579
This commit is contained in:
commit
0c22a47a3e
@ -210,9 +210,14 @@ class SongSelectImport(object):
|
|||||||
for author_name in song['authors']:
|
for author_name in song['authors']:
|
||||||
author = self.db_manager.get_object_filtered(Author, Author.display_name == author_name)
|
author = self.db_manager.get_object_filtered(Author, Author.display_name == author_name)
|
||||||
if not author:
|
if not author:
|
||||||
author = Author.populate(first_name=author_name.rsplit(' ', 1)[0],
|
name_parts = author_name.rsplit(' ', 1)
|
||||||
last_name=author_name.rsplit(' ', 1)[1],
|
first_name = name_parts[0]
|
||||||
display_name=author_name)
|
if len(name_parts) == 1:
|
||||||
|
last_name = ''
|
||||||
|
else:
|
||||||
|
last_name = name_parts[1]
|
||||||
|
author = Author.populate(first_name=first_name, last_name=last_name, display_name=author_name)
|
||||||
db_song.add_author(author)
|
db_song.add_author(author)
|
||||||
self.db_manager.save_object(db_song)
|
self.db_manager.save_object(db_song)
|
||||||
return db_song
|
return db_song
|
||||||
|
|
||||||
|
@ -402,6 +402,42 @@ class TestSongSelectImport(TestCase, TestMixin):
|
|||||||
self.assertEqual(0, MockedAuthor.populate.call_count, 'A new author should not have been instantiated')
|
self.assertEqual(0, MockedAuthor.populate.call_count, 'A new author should not have been instantiated')
|
||||||
self.assertEqual(1, len(result.authors_songs), 'There should only be one author')
|
self.assertEqual(1, len(result.authors_songs), 'There should only be one author')
|
||||||
|
|
||||||
|
@patch('openlp.plugins.songs.lib.songselect.clean_song')
|
||||||
|
@patch('openlp.plugins.songs.lib.songselect.Author')
|
||||||
|
def save_song_unknown_author_test(self, MockedAuthor, mocked_clean_song):
|
||||||
|
"""
|
||||||
|
Test that saving a song with an author name of only one word performs the correct actions
|
||||||
|
"""
|
||||||
|
# GIVEN: A song to save, and some mocked out objects
|
||||||
|
song_dict = {
|
||||||
|
'title': 'Arky Arky',
|
||||||
|
'authors': ['Unknown'],
|
||||||
|
'verses': [
|
||||||
|
{'label': 'Verse 1', 'lyrics': 'The Lord told Noah: there\'s gonna be a floody, floody'},
|
||||||
|
{'label': 'Chorus 1', 'lyrics': 'So, rise and shine, and give God the glory, glory'},
|
||||||
|
{'label': 'Verse 2', 'lyrics': 'The Lord told Noah to build him an arky, arky'}
|
||||||
|
],
|
||||||
|
'copyright': 'Public Domain',
|
||||||
|
'ccli_number': '123456'
|
||||||
|
}
|
||||||
|
MockedAuthor.display_name.__eq__.return_value = False
|
||||||
|
mocked_db_manager = MagicMock()
|
||||||
|
mocked_db_manager.get_object_filtered.return_value = None
|
||||||
|
importer = SongSelectImport(mocked_db_manager)
|
||||||
|
|
||||||
|
# WHEN: The song is saved to the database
|
||||||
|
result = importer.save_song(song_dict)
|
||||||
|
|
||||||
|
# THEN: The return value should be a Song class and the mocked_db_manager should have been called
|
||||||
|
self.assertIsInstance(result, Song, 'The returned value should be a Song object')
|
||||||
|
mocked_clean_song.assert_called_with(mocked_db_manager, result)
|
||||||
|
self.assertEqual(2, mocked_db_manager.save_object.call_count,
|
||||||
|
'The save_object() method should have been called twice')
|
||||||
|
mocked_db_manager.get_object_filtered.assert_called_with(MockedAuthor, False)
|
||||||
|
MockedAuthor.populate.assert_called_with(first_name='Unknown', last_name='',
|
||||||
|
display_name='Unknown')
|
||||||
|
self.assertEqual(1, len(result.authors_songs), 'There should only be one author')
|
||||||
|
|
||||||
|
|
||||||
class TestSongSelectForm(TestCase, TestMixin):
|
class TestSongSelectForm(TestCase, TestMixin):
|
||||||
"""
|
"""
|
||||||
|
Loading…
Reference in New Issue
Block a user