openlp-core team mailing list archive

[Samuel Mehrbrodt <s.mehrbrodt@xxxxxxxxx>]

Samuel Mehrbrodt has proposed merging lp:~sam92/openlp/bug-1313538 into lp:openlp.

Requested reviews:
  Raoul Snyman (raoul-snyman)
  Tim Bentley (trb143)
Related bugs:
  Bug #1313538 in OpenLP: "Song duplicates added when using different author types for the same author"
  https://bugs.launchpad.net/openlp/+bug/1313538

For more details, see:
https://code.launchpad.net/~sam92/openlp/bug-1313538/+merge/220489

Song duplicates added when using different author types for the same author

lp:~sam92/openlp/bug-1313538 (revision 2382)
[SUCCESS] http://ci.openlp.org/job/Branch-01-Pull/451/
[SUCCESS] http://ci.openlp.org/job/Branch-02-Functional-Tests/407/
[SUCCESS] http://ci.openlp.org/job/Branch-03-Interface-Tests/352/
[SUCCESS] http://ci.openlp.org/job/Branch-04-Windows_Tests/313/
[SUCCESS] http://ci.openlp.org/job/Branch-05a-Code_Analysis/215/
[SUCCESS] http://ci.openlp.org/job/Branch-05b-Test_Coverage/89/
-- 
https://code.launchpad.net/~sam92/openlp/bug-1313538/+merge/220489
Your team OpenLP Core is subscribed to branch lp:openlp.

=== modified file 'openlp/plugins/songs/lib/mediaitem.py'
--- openlp/plugins/songs/lib/mediaitem.py	2014-05-10 06:43:18 +0000
+++ openlp/plugins/songs/lib/mediaitem.py	2014-05-21 15:39:31 +0000
@@ -527,15 +527,7 @@
         add_song = True
         if search_results:
             for song in search_results:
-                author_list = item.data_string['authors']
-                same_authors = True
-                for author in song.authors:
-                    if author.display_name in author_list:
-                        author_list = author_list.replace(author.display_name, '', 1)
-                    else:
-                        same_authors = False
-                        break
-                if same_authors and author_list.strip(', ') == '':
+                if self._authors_match(song, item.data_string['authors']):
                     add_song = False
                     edit_id = song.id
                     break
@@ -561,6 +553,23 @@
         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.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

=== modified file 'tests/functional/openlp_plugins/songs/test_mediaitem.py'
--- tests/functional/openlp_plugins/songs/test_mediaitem.py	2014-05-03 10:53:51 +0000
+++ tests/functional/openlp_plugins/songs/test_mediaitem.py	2014-05-21 15:39:31 +0000
@@ -153,3 +153,36 @@
 
         # 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):
+        """
+        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"
+
+        # WHEN: Checking for matching
+        result = self.media_item._authors_match(song, authors_str)
+
+        # THEN: They should match
+        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")