Merge branch 'sort_authors_on_export' into 'master'

Sort the authors when exporting the songs

See merge request openlp/openlp!7

openlp/plugins/songs/forms/songexportform.py

@@ -32,6 +32,7 @@ from openlp.core.common.registry import Registry
 from openlp.core.common.settings import Settings
 from openlp.core.lib import create_separated_list
 from openlp.core.lib.ui import critical_error_message_box
+from openlp.core.common.i18n import get_natural_key
 from openlp.core.widgets.edits import PathEdit
 from openlp.core.widgets.enums import PathEditType
 from openlp.core.widgets.wizard import OpenLPWizard, WizardStrings
@@ -219,8 +220,11 @@ class SongExportForm(OpenLPWizard):
             # No need to export temporary songs.
             if song.temporary:
                 continue
-            authors = create_separated_list([author.display_name for author in song.authors])
+
-            title = '{title} ({author})'.format(title=song.title, author=authors)
+            authors = [author.display_name for author in song.authors]
+            authors.sort(key=get_natural_key)
+            title = '{title} ({author})'.format(title=song.title,
+                                                author=create_separated_list(authors))
             item = QtWidgets.QListWidgetItem(title)
             item.setData(QtCore.Qt.UserRole, song)
             item.setFlags(QtCore.Qt.ItemIsSelectable | QtCore.Qt.ItemIsUserCheckable | QtCore.Qt.ItemIsEnabled)

tests/functional/openlp_plugins/songs/test_openlyricsexport.py

@@ -76,3 +76,40 @@ class TestOpenLyricsExport(TestCase, TestMixin):
                 title=song.title, display_name=author.display_name)).exists() is True
             assert (self.temp_folder / '{title} ({display_name})-1.xml'.format(
                 title=song.title, display_name=author.display_name)).exists() is True
+
+        def test_export_sort_of_authers_filename(self):
+            """
+            Test that files is not overwritten if songs has same title and author
+            """
+            # GIVEN: A mocked song_to_xml, 1 mocked songs, a mocked application and an OpenLyricsExport instance
+            with patch('openlp.plugins.songs.lib.openlyricsexport.OpenLyrics.song_to_xml') as mocked_song_to_xml:
+                mocked_song_to_xml.return_value = '<?xml version="1.0" encoding="UTF-8"?>\n<empty/>'
+                authorA = MagicMock()
+                authorA.display_name = 'a Author'
+                authorB = MagicMock()
+                authorB.display_name = 'b Author'
+                songA = MagicMock()
+                songA.authors = [authorA, authorB]
+                songA.title = 'Test Title'
+                songB = MagicMock()
+                songB.authors = [authorB, authorA]
+                songB.title = 'Test Title'
+
+                parent = MagicMock()
+                parent.stop_export_flag = False
+                mocked_application_object = MagicMock()
+                Registry().register('application', mocked_application_object)
+                ol_export = OpenLyricsExport(parent, [songA, songB], self.temp_folder)
+
+                # WHEN: Doing the export
+                ol_export.do_export()
+
+                # THEN: The exporter orders authers
+                assert (self.temp_folder / '{title} ({display_name}).xml'.format(
+                    title=song.title,
+                    display_name=", ".join([authorA.display_name, authorB.display_name])
+                )).exists() is True
+                assert (self.temp_folder / '{title} ({display_name})-1.xml'.format(
+                    title=song.title,
+                    display_name=", ".join([authorA.display_name, authorB.display_name])
+                )).exists() is True