Merge lp:~trb143/openlp/bug-772523 into lp:openlp

[Tim Bentley]

This patch changes your song database so back it up!

When a service is imported at present and we do not wish to save the songs we do not. This prevents the user from editing songs.
This change adds a "temporary" flag on the song database to allow those songs to be save and edited bit not via the plugin as the search will not show them.
On exit the songs will be deleted.

The code works until a service is loaded and then you get sqlalchemy errors.
I have no idea what is wrong but some help would be appreciated.

[Raoul Snyman (raoul-snyman)]

Well, first remove your print statement.

review: Needs Fixing

[Raoul Snyman (raoul-snyman)]

You will need to set the default of the new column, something like this:

110 Column(u'temporary', types.Boolean(), default=False)\
111 .create(table=tables[u'songs'])
        connection = metadata.bind.connect()
        update_statement = tables[u'songs'].update().values(temporary=False)
        connection.execute(update_statement)

[lp:~trb143/openlp/bug-772523]

1772. By Tim Bentley on 2011-12-05

Fix up song import code

1773. By Tim Bentley on 2011-12-05

Head

1774. By Tim Bentley on 2011-12-08

Fixes

1775. By Tim Bentley on 2011-12-08

Head

1776. By Tim Bentley on 2011-12-10

Fixes and notifications

1777. By Tim Bentley on 2011-12-10

Lost temporary edit flag

1778. By Tim Bentley on 2011-12-10

clean up service

1779. By Tim Bentley on 2011-12-12

Head

1780. By Tim Bentley on 2011-12-12

Fix review comments

1781. By Tim Bentley on 2011-12-12

Fix review comments

Unmerged revisions

=== modified file 'openlp/core/lib/db.py'
--- openlp/core/lib/db.py	2011-12-03 12:51:40 +0000
+++ openlp/core/lib/db.py	2011-12-05 19:51:24 +0000
@@ -358,10 +358,17 @@
 
     def delete_all_objects(self, object_class, filter_clause=None):
         """
-        Delete all object records
+        Delete all object records.
+        This method should only be used for simple tables and not ones with
+        relationships.  The relationships are not deleted from the database and
+        this will lead to database corruptions.
 
         ``object_class``
             The type of object to delete
+
+        ``filter_clause``
+            The filter governing selection of objects to return. Defaults to
+            None.
         """
         try:
             query = self.session.query(object_class)

=== modified file 'openlp/plugins/songs/forms/songexportform.py'
--- openlp/plugins/songs/forms/songexportform.py	2011-08-26 15:48:58 +0000
+++ openlp/plugins/songs/forms/songexportform.py	2011-12-05 19:51:24 +0000
@@ -252,6 +252,9 @@
         songs = self.plugin.manager.get_all_objects(Song)
         songs.sort(cmp=locale.strcoll, key=lambda song: song.title.lower())
         for song in songs:
+            # No need to export temporary songs.
+            if song.temporary:
+                continue
             authors = u', '.join([author.display_name
                 for author in song.authors])
             title = u'%s (%s)' % (unicode(song.title), authors)

=== modified file 'openlp/plugins/songs/lib/db.py'
--- openlp/plugins/songs/lib/db.py	2011-08-26 23:04:54 +0000
+++ openlp/plugins/songs/lib/db.py	2011-12-05 19:51:24 +0000
@@ -199,7 +199,8 @@
         Column(u'search_lyrics', types.UnicodeText, nullable=False),
         Column(u'create_date', types.DateTime(), default=func.now()),
         Column(u'last_modified', types.DateTime(), default=func.now(),
-            onupdate=func.now())
+            onupdate=func.now()),
+        Column(u'temporary', types.Boolean(), default=False)
     )
 
     # Definition of the "topics" table

=== modified file 'openlp/plugins/songs/lib/mediaitem.py'
--- openlp/plugins/songs/lib/mediaitem.py	2011-11-24 22:34:27 +0000
+++ openlp/plugins/songs/lib/mediaitem.py	2011-12-05 19:51:24 +0000
@@ -270,6 +270,9 @@
         searchresults.sort(
             cmp=locale.strcoll, key=lambda song: song.title.lower())
         for song in searchresults:
+            # Do not display temporary songs
+            if song.temporary:
+                continue
             author_list = [author.display_name for author in song.authors]
             song_title = unicode(song.title)
             song_detail = u'%s (%s)' % (song_title, u', '.join(author_list))
@@ -286,6 +289,9 @@
         self.listView.clear()
         for author in searchresults:
             for song in author.songs:
+                # Do not display temporary songs
+                if song.temporary:
+                    continue
                 song_detail = u'%s (%s)' % (author.display_name, song.title)
                 song_name = QtGui.QListWidgetItem(song_detail)
                 song_name.setData(QtCore.Qt.UserRole, QtCore.QVariant(song.id))
@@ -552,16 +558,20 @@
                 # If there's any backing tracks, copy them over.
                 if len(item.background_audio) > 0:
                     self._updateBackgroundAudio(song, item)
-        if add_song and self.addSongFromService:
+        if add_song:
             song = self.openLyrics.xml_to_song(item.xml_version)
             # If there's any backing tracks, copy them over.
             if len(item.background_audio) > 0:
                 self._updateBackgroundAudio(song, item)
             editId = song.id
             self.onSearchTextButtonClick()
-        else:
+        elif not self.addSongFromService:
             # Make sure we temporary import formatting tags.
-            self.openLyrics.xml_to_song(item.xml_version, True)
+            song = self.openLyrics.xml_to_song(item.xml_version, True)
+            # If there's any backing tracks, copy them over.
+            if len(item.background_audio) > 0:
+                self._updateBackgroundAudio(song, item)
+            editId = song.id
         # Update service with correct song id.
         if editId:
             Receiver.send_message(u'service_item_update',

=== modified file 'openlp/plugins/songs/lib/upgrade.py'
--- openlp/plugins/songs/lib/upgrade.py	2011-12-02 20:17:57 +0000
+++ openlp/plugins/songs/lib/upgrade.py	2011-12-05 19:51:24 +0000
@@ -33,7 +33,9 @@
 from sqlalchemy.sql.expression import func
 from migrate.changeset.constraint import ForeignKeyConstraint
 
-__version__ = 2
+from openlp.plugins.songs.lib.db import Song
+
+__version__ = 3
 
 def upgrade_setup(metadata):
     """
@@ -86,3 +88,12 @@
     Column(u'last_modified', types.DateTime(), default=func.now())\
         .create(table=tables[u'songs'])
 
+def upgrade_3(session, metadata, tables):
+    """
+    Version 3 upgrade.
+
+    This upgrade adds a temporary song flag to the songs table
+    """
+    Column(u'temporary', types.Boolean(), default=False)\
+        .create(table=tables[u'songs'])
+

=== modified file 'openlp/plugins/songs/lib/xml.py'
--- openlp/plugins/songs/lib/xml.py	2011-10-09 19:51:44 +0000
+++ openlp/plugins/songs/lib/xml.py	2011-12-05 19:51:24 +0000
@@ -346,7 +346,7 @@
                     lines_element.set(u'break', u'optional')
         return self._extract_xml(song_xml)
 
-    def xml_to_song(self, xml, parse_and_not_save=False):
+    def xml_to_song(self, xml, parse_and_temporary_save=False):
         """
         Create and save a song from OpenLyrics format xml to the database. Since
         we also export XML from external sources (e. g. OpenLyrics import), we
@@ -355,9 +355,9 @@
         ``xml``
             The XML to parse (unicode).
 
-        ``parse_and_not_save``
-            Switch to skip processing the whole song and to prevent storing the
-            songs to the database. Defaults to ``False``.
+        ``parse_and_temporary_save``
+            Switch to skip processing the whole song and storing the songs in
+            the database with a temporary flag. Defaults to ``False``.
         """
         # No xml get out of here.
         if not xml:
@@ -371,14 +371,13 @@
             return None
         # Formatting tags are new in OpenLyrics 0.8
         if float(song_xml.get(u'version')) > 0.7:
-            self._process_formatting_tags(song_xml, parse_and_not_save)
-        if parse_and_not_save:
-            return
+            self._process_formatting_tags(song_xml, parse_and_temporary_save)
         song = Song()
         # Values will be set when cleaning the song.
         song.search_lyrics = u''
         song.verse_order = u''
         song.search_title = u''
+        song.temporary = parse_and_temporary_save
         self._process_copyright(properties, song)
         self._process_cclinumber(properties, song)
         self._process_titles(properties, song)

=== modified file 'openlp/plugins/songs/songsplugin.py'
--- openlp/plugins/songs/songsplugin.py	2011-12-03 13:32:19 +0000
+++ openlp/plugins/songs/songsplugin.py	2011-12-05 19:51:24 +0000
@@ -264,6 +264,11 @@
         Time to tidy up on exit
         """
         log.info(u'Songs Finalising')
+        # Remove temporary songs
+        songs = self.manager.get_all_objects(Song, Song.temporary == True)
+        for song in songs:
+            self.manager.delete_object(Song, song.id)
+        # Clean up files and connections
         self.manager.finalise()
         self.songImportItem.setVisible(False)
         self.songExportItem.setVisible(False)