openlp-core team mailing list archive

[Jonathan Corwin <j@xxxxxxxxxxxx>]

Jonathan Corwin has proposed merging lp:~j-corwin/openlp/bug-794041 into lp:openlp.

Requested reviews:
  OpenLP Core (openlp-core)
Related bugs:
  Bug #780372 in OpenLP: "Edit item in song edit in service side not showing"
  https://bugs.launchpad.net/openlp/+bug/780372
  Bug #794041 in OpenLP: "Long titles cause OpenLyrics export to crash"
  https://bugs.launchpad.net/openlp/+bug/794041

For more details, see:
https://code.launchpad.net/~j-corwin/openlp/bug-794041/+merge/65141

1. For openlyrics export, restrict the length of the filename to ensure it isn't too long for the filesystem.

2. When adding a song from a loaded service file, the author list string was being split by comma and the count compared to the number of authors against the db song. This gave a false negative if one of the individual authors contained a comma. Change the author comparison technique.
-- 
https://code.launchpad.net/~j-corwin/openlp/bug-794041/+merge/65141
Your team OpenLP Core is requested to review the proposed merge of lp:~j-corwin/openlp/bug-794041 into lp:openlp.

=== modified file 'openlp/plugins/songs/lib/mediaitem.py'
--- openlp/plugins/songs/lib/mediaitem.py	2011-06-12 16:02:52 +0000
+++ openlp/plugins/songs/lib/mediaitem.py	2011-06-19 22:23:27 +0000
@@ -467,23 +467,20 @@
             search_results = self.plugin.manager.get_all_objects(Song,
                 Song.search_title == item.data_string[u'title'],
                 Song.search_title.asc())
-        author_list = item.data_string[u'authors'].split(u', ')
         editId = 0
         add_song = True
         if search_results:
             for song in search_results:
+                author_list = item.data_string[u'authors']
                 same_authors = True
-                # If the author counts are different, we do not have to do any
-                # further checking.
-                if len(song.authors) == len(author_list):
-                    for author in song.authors:
-                        if author.display_name not in author_list:
-                            same_authors = False
-                else:
-                    same_authors = False
-                # All authors are the same, so we can stop here and the song
-                # does not have to be saved.
-                if same_authors:
+                for author in song.authors:
+                    if author.display_name in author_list:
+                        author_list = author_list.replace(author.display_name,
+                            u'', 1)
+                    else:
+                        same_authors = False
+                        break
+                if same_authors and author_list.strip(u', ') == u'':
                     add_song = False
                     editId = song.id
                     break

=== modified file 'openlp/plugins/songs/lib/openlyricsexport.py'
--- openlp/plugins/songs/lib/openlyricsexport.py	2011-06-12 16:02:52 +0000
+++ openlp/plugins/songs/lib/openlyricsexport.py	2011-06-19 22:23:27 +0000
@@ -70,10 +70,12 @@
                 song.title)
             xml = openLyrics.song_to_xml(song)
             tree = etree.ElementTree(etree.fromstring(xml))
-            filename = u'%s (%s).xml' % (song.title,
+            filename = u'%s (%s)' % (song.title,
                 u', '.join([author.display_name for author in song.authors]))
             filename = re.sub(
                 r'[/\\?*|<>\[\]":<>+%]+', u'_', filename).strip(u'_')
+            # Ensure the filename isn't too long for some filesystems
+            filename = u'%s.xml' % filename[0:250 - len(self.save_path)]
             # Pass a file object, because lxml does not cope with some special
             # characters in the path (see lp:757673 and lp:744337).
             tree.write(open(os.path.join(self.save_path, filename), u'w'),