From 13d9e77c08bafd4a939a537921ab093130a78ee4 Mon Sep 17 00:00:00 2001 From: Samuel Mehrbrodt Date: Wed, 7 May 2014 13:56:14 +0200 Subject: [PATCH 1/8] Fix bug 1313538 Fixes: https://launchpad.net/bugs/1313538 --- openlp/plugins/songs/lib/mediaitem.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/openlp/plugins/songs/lib/mediaitem.py b/openlp/plugins/songs/lib/mediaitem.py index 288711b06..5cb8af47a 100644 --- a/openlp/plugins/songs/lib/mediaitem.py +++ b/openlp/plugins/songs/lib/mediaitem.py @@ -524,15 +524,15 @@ class SongMediaItem(MediaManagerItem): add_song = True if search_results: for song in search_results: - author_list = item.data_string['authors'] + author_list = item.data_string['authors'].split(', ') same_authors = True for author in song.authors: if author.display_name in author_list: - author_list = author_list.replace(author.display_name, '', 1) + author_list = author_list.remove(author.display_name) else: same_authors = False break - if same_authors and author_list.strip(', ') == '': + if same_authors and not author_list: add_song = False edit_id = song.id break From cca1ce57f370e6b7ad7d71c90f185f08371707bb Mon Sep 17 00:00:00 2001 From: Samuel Mehrbrodt Date: Wed, 7 May 2014 14:07:13 +0200 Subject: [PATCH 2/8] Add test --- tests/functional/openlp_core_lib/test_ui.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/functional/openlp_core_lib/test_ui.py b/tests/functional/openlp_core_lib/test_ui.py index 025b1a638..8fdfabd14 100644 --- a/tests/functional/openlp_core_lib/test_ui.py +++ b/tests/functional/openlp_core_lib/test_ui.py @@ -146,6 +146,20 @@ class TestUi(TestCase): self.assertEqual('combo1', combo.objectName()) self.assertEqual(QtGui.QComboBox.AdjustToMinimumContentsLength, combo.sizeAdjustPolicy()) + def test_create_widget_action(self): + """ + Test creating an action for a widget + """ + # GIVEN: A button + button = QtGui.QPushButton() + + # WHEN: We call the function + action = create_widget_action(button, 'some action') + + # THEN: The action should be returned + self.assertIsInstance(action, QtGui.QAction) + self.assertEqual(action.objectName(), 'some action') + def test_create_action(self): """ Test creating an action From 9b173dc0fef46f37aee095a4a6343eed0121acd5 Mon Sep 17 00:00:00 2001 From: Samuel Mehrbrodt Date: Tue, 13 May 2014 11:27:22 +0200 Subject: [PATCH 3/8] Refactor author match check --- openlp/plugins/songs/lib/mediaitem.py | 28 ++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/openlp/plugins/songs/lib/mediaitem.py b/openlp/plugins/songs/lib/mediaitem.py index 5cb8af47a..04fa5a77a 100644 --- a/openlp/plugins/songs/lib/mediaitem.py +++ b/openlp/plugins/songs/lib/mediaitem.py @@ -524,15 +524,7 @@ class SongMediaItem(MediaManagerItem): add_song = True if search_results: for song in search_results: - author_list = item.data_string['authors'].split(', ') - same_authors = True - for author in song.authors: - if author.display_name in author_list: - author_list = author_list.remove(author.display_name) - else: - same_authors = False - break - if same_authors and not author_list: + if self._authors_match(song, item.data_string['authors']): add_song = False edit_id = song.id break @@ -558,6 +550,24 @@ class SongMediaItem(MediaManagerItem): self.generate_footer(item, song) return item + def _authors_match(self, song, authors): + """ + Checks whether authors from a song in the database match the authors of the song to be imported. + + :param song: A list of authors from the song in the database + :param authors: A string with authors from the song to be imported + :return: True when Authors do match, else false. + """ + author_list = authors.split(', ') + for author in song.authors: + if author.display_name in author_list: + author_list = author_list.remove(author.display_name) + else: + return False + # List must be empty at the end + return not author_list + + def search(self, string, show_error): """ Search for some songs From 97b603a2d6747dadbf1a9e492bd26e5c73857e8f Mon Sep 17 00:00:00 2001 From: Samuel Mehrbrodt Date: Tue, 13 May 2014 11:44:19 +0200 Subject: [PATCH 4/8] Add test for author matching --- openlp/plugins/songs/lib/mediaitem.py | 5 ++-- .../openlp_plugins/songs/test_mediaitem.py | 29 +++++++++++++++++++ 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/openlp/plugins/songs/lib/mediaitem.py b/openlp/plugins/songs/lib/mediaitem.py index 04fa5a77a..e51743413 100644 --- a/openlp/plugins/songs/lib/mediaitem.py +++ b/openlp/plugins/songs/lib/mediaitem.py @@ -556,18 +556,17 @@ class SongMediaItem(MediaManagerItem): :param song: A list of authors from the song in the database :param authors: A string with authors from the song to be imported - :return: True when Authors do match, else false. + :return: True when Authors do match, else False. """ author_list = authors.split(', ') for author in song.authors: if author.display_name in author_list: - author_list = author_list.remove(author.display_name) + author_list.remove(author.display_name) else: return False # List must be empty at the end return not author_list - def search(self, string, show_error): """ Search for some songs diff --git a/tests/functional/openlp_plugins/songs/test_mediaitem.py b/tests/functional/openlp_plugins/songs/test_mediaitem.py index 308881c2e..359e27a0f 100644 --- a/tests/functional/openlp_plugins/songs/test_mediaitem.py +++ b/tests/functional/openlp_plugins/songs/test_mediaitem.py @@ -128,3 +128,32 @@ class TestMediaItem(TestCase, TestMixin): # THEN: I would get an amended footer string self.assertEqual(service_item.raw_footer, ['My Song', 'My copyright', 'CCLI License: 4321'], 'The array should be returned correctly with a song, an author, copyright and amended ccli') + + def match_authors_test(self): + """ + Test the author matching when importing a song from a service + """ + # GIVEN: A song and a string with authors + song = MagicMock() + song.authors = [] + author = MagicMock() + author.display_name = "Hans Wurst" + song.authors.append(author) + author2 = MagicMock() + author2.display_name = "Max Mustermann" + song.authors.append(author2) + # There are occasions where an author appears twice in a song (with different types). + # We need to make sure that this case works (lp#1313538) + author3 = MagicMock() + author3.display_name = "Max Mustermann" + song.authors.append(author3) + authors_str = "Hans Wurst, Max Mustermann, Max Mustermann" + authors_str_wrong = "Hans Wurst, Max Mustermann" + + # WHEN: Checking for matching + correct_result = self.media_item._authors_match(song, authors_str) + wrong_result = self.media_item._authors_match(song, authors_str_wrong) + + # THEN: They should match + self.assertTrue(correct_result) + self.assertFalse(wrong_result) From a75790d6997da54334aa15a2a9cecedc9e274f3d Mon Sep 17 00:00:00 2001 From: Samuel Mehrbrodt Date: Tue, 13 May 2014 11:54:12 +0200 Subject: [PATCH 5/8] Indentation --- tests/functional/openlp_plugins/songs/test_mediaitem.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/functional/openlp_plugins/songs/test_mediaitem.py b/tests/functional/openlp_plugins/songs/test_mediaitem.py index d21342926..1b4374bcd 100644 --- a/tests/functional/openlp_plugins/songs/test_mediaitem.py +++ b/tests/functional/openlp_plugins/songs/test_mediaitem.py @@ -128,7 +128,7 @@ class TestMediaItem(TestCase, TestMixin): self.assertEqual(service_item.raw_footer, ['My Song', 'My copyright', 'CCLI License: 4321'], 'The array should be returned correctly with a song, an author, copyright and amended ccli') - def build_song_footer_base_songbook_test(self): + def build_song_footer_base_songbook_test(self): """ Test build songs footer with basic song and a songbook """ From d6afa4905556febbc49d1ce51e02697e3c7e4eea Mon Sep 17 00:00:00 2001 From: Samuel Mehrbrodt Date: Tue, 13 May 2014 19:52:08 +0200 Subject: [PATCH 6/8] Test fixes --- .../openlp_plugins/songs/test_mediaitem.py | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/tests/functional/openlp_plugins/songs/test_mediaitem.py b/tests/functional/openlp_plugins/songs/test_mediaitem.py index 1b4374bcd..b3c097268 100644 --- a/tests/functional/openlp_plugins/songs/test_mediaitem.py +++ b/tests/functional/openlp_plugins/songs/test_mediaitem.py @@ -173,12 +173,16 @@ class TestMediaItem(TestCase, TestMixin): author3.display_name = "Max Mustermann" song.authors.append(author3) authors_str = "Hans Wurst, Max Mustermann, Max Mustermann" - authors_str_wrong = "Hans Wurst, Max Mustermann" # WHEN: Checking for matching - correct_result = self.media_item._authors_match(song, authors_str) - wrong_result = self.media_item._authors_match(song, authors_str_wrong) + result = self.media_item._authors_match(song, authors_str) # THEN: They should match - self.assertTrue(correct_result) - self.assertFalse(wrong_result) + self.assertTrue(result, "Authors should match") + + # WHEN: An author is missing in the string + authors_str = "Hans Wurst, Max Mustermann" + result = self.media_item._authors_match(song, authors_str) + + # THEN: They should not match + self.assertFalse(result, "Authors should not match") From 5cd2f7ebff3494d58811c914c429c07777249955 Mon Sep 17 00:00:00 2001 From: Samuel Mehrbrodt Date: Wed, 21 May 2014 17:31:02 +0200 Subject: [PATCH 7/8] Remove tests --- tests/functional/openlp_core_lib/test_ui.py | 29 --------------------- 1 file changed, 29 deletions(-) diff --git a/tests/functional/openlp_core_lib/test_ui.py b/tests/functional/openlp_core_lib/test_ui.py index 604b75d62..f1b8ee17a 100644 --- a/tests/functional/openlp_core_lib/test_ui.py +++ b/tests/functional/openlp_core_lib/test_ui.py @@ -129,35 +129,6 @@ class TestUi(TestCase): self.assertEqual('my_btn', btn.objectName()) self.assertTrue(btn.isEnabled()) - def test_create_horizontal_adjusting_combo_box(self): - """ - Test creating a horizontal adjusting combo box - """ - # GIVEN: A dialog - dialog = QtGui.QDialog() - - # WHEN: We create the combobox - combo = create_horizontal_adjusting_combo_box(dialog, 'combo1') - - # THEN: We should get a ComboBox - self.assertIsInstance(combo, QtGui.QComboBox) - self.assertEqual('combo1', combo.objectName()) - self.assertEqual(QtGui.QComboBox.AdjustToMinimumContentsLength, combo.sizeAdjustPolicy()) - - def test_create_widget_action(self): - """ - Test creating an action for a widget - """ - # GIVEN: A button - button = QtGui.QPushButton() - - # WHEN: We call the function - action = create_widget_action(button, 'some action') - - # THEN: The action should be returned - self.assertIsInstance(action, QtGui.QAction) - self.assertEqual(action.objectName(), 'some action') - def test_create_action(self): """ Test creating an action From dd9d535f81227adb5a6bc4179b1bca81b908df64 Mon Sep 17 00:00:00 2001 From: Samuel Mehrbrodt Date: Wed, 21 May 2014 21:55:16 +0200 Subject: [PATCH 8/8] Split test in two methods --- .../openlp_plugins/songs/test_mediaitem.py | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/tests/functional/openlp_plugins/songs/test_mediaitem.py b/tests/functional/openlp_plugins/songs/test_mediaitem.py index b3c097268..bc22a4577 100644 --- a/tests/functional/openlp_plugins/songs/test_mediaitem.py +++ b/tests/functional/openlp_plugins/songs/test_mediaitem.py @@ -154,7 +154,7 @@ class TestMediaItem(TestCase, TestMixin): # THEN: The songbook should be in the footer self.assertEqual(service_item.raw_footer, ['My Song', 'My copyright', 'My songbook #12']) - def match_authors_test(self): + def authors_match_test(self): """ Test the author matching when importing a song from a service """ @@ -180,6 +180,22 @@ class TestMediaItem(TestCase, TestMixin): # THEN: They should match self.assertTrue(result, "Authors should match") + def authors_dont_match_test(self): + # GIVEN: A song and a string with authors + song = MagicMock() + song.authors = [] + author = MagicMock() + author.display_name = "Hans Wurst" + song.authors.append(author) + author2 = MagicMock() + author2.display_name = "Max Mustermann" + song.authors.append(author2) + # There are occasions where an author appears twice in a song (with different types). + # We need to make sure that this case works (lp#1313538) + author3 = MagicMock() + author3.display_name = "Max Mustermann" + song.authors.append(author3) + # WHEN: An author is missing in the string authors_str = "Hans Wurst, Max Mustermann" result = self.media_item._authors_match(song, authors_str)