Fix song validating and saving process

openlp/plugins/songs/forms/editsongform.py

@@ -332,7 +332,7 @@ class EditSongForm(QtGui.QDialog, Ui_EditSongDialog):
                 else:
                     author = Author.populate(first_name=text.rsplit(u' ', 1)[0],
                         last_name=text.rsplit(u' ', 1)[1], display_name=text)
-                self.manager.save_object(author, False)
+                self.manager.save_object(author)
                 author_item = QtGui.QListWidgetItem(
                     unicode(author.display_name))
                 author_item.setData(QtCore.Qt.UserRole,
@@ -386,7 +386,7 @@ class EditSongForm(QtGui.QDialog, Ui_EditSongDialog):
                 QtGui.QMessageBox.Yes | QtGui.QMessageBox.No,
                 QtGui.QMessageBox.Yes) == QtGui.QMessageBox.Yes:
                 topic = Topic.populate(name=text)
-                self.manager.save_object(topic, False)
+                self.manager.save_object(topic)
                 topic_item = QtGui.QListWidgetItem(unicode(topic.name))
                 topic_item.setData(QtCore.Qt.UserRole,
                     QtCore.QVariant(topic.id))
@@ -524,12 +524,13 @@ class EditSongForm(QtGui.QDialog, Ui_EditSongDialog):
 
     def _validate_song(self):
         """
-        Check the validity of the form. Only display the 'save' if the data
+        Check the validity of the song.
-        can be saved.
         """
+        # This checks data in the form *not* self.song.  self.song is still
+        # None at this point.
         log.debug(u'Validate Song')
         # Lets be nice and assume the data is correct.
-        if len(self.titleEdit.displayText()) == 0:
+        if not self.titleEdit.text():
             self.songTabWidget.setCurrentIndex(0)
             self.titleEdit.setFocus()
             criticalErrorMessageBox(
@@ -550,9 +551,9 @@ class EditSongForm(QtGui.QDialog, Ui_EditSongDialog):
                 message=translate('SongsPlugin.EditSongForm',
                 'You need to have an author for this song.'))
             return False
-        if self.song.verse_order:
+        if self.verseOrderEdit.text():
             order = []
-            order_names = self.song.verse_order.split()
+            order_names = self.verseOrderEdit.text().split()
             for item in order_names:
                 if len(item) == 1:
                     order.append(item.lower() + u'1')
@@ -593,6 +594,19 @@ class EditSongForm(QtGui.QDialog, Ui_EditSongDialog):
                         QtGui.QMessageBox.Yes | QtGui.QMessageBox.No)
                     if answer == QtGui.QMessageBox.No:
                         return False
+        item = int(self.songBookComboBox.currentIndex())
+        text = unicode(self.songBookComboBox.currentText())
+        if self.songBookComboBox.findText(text, QtCore.Qt.MatchExactly) < 0:
+            if QtGui.QMessageBox.question(self,
+                translate('SongsPlugin.EditSongForm', 'Add Book'),
+                translate('SongsPlugin.EditSongForm', 'This song book does '
+                'not exist, do you want to add it?'),
+                QtGui.QMessageBox.Yes | QtGui.QMessageBox.No,
+                QtGui.QMessageBox.Yes) == QtGui.QMessageBox.Yes:
+                book = Book.populate(name=text, publisher=u'')
+                self.manager.save_object(book)
+            else:
+                return False
         return True
 
     def onCopyrightInsertButtonTriggered(self):
@@ -653,33 +667,25 @@ class EditSongForm(QtGui.QDialog, Ui_EditSongDialog):
         """
         log.debug(u'accept')
         self.clearCaches()
-        if not self.song:
+        if self._validate_song():
-            self.song = Song()
+            self.saveSong()
-        item = int(self.songBookComboBox.currentIndex())
-        text = unicode(self.songBookComboBox.currentText())
-        if self.songBookComboBox.findText(text, QtCore.Qt.MatchExactly) < 0:
-            if QtGui.QMessageBox.question(self,
-                translate('SongsPlugin.EditSongForm', 'Add Book'),
-                translate('SongsPlugin.EditSongForm', 'This song book does '
-                'not exist, do you want to add it?'),
-                QtGui.QMessageBox.Yes | QtGui.QMessageBox.No,
-                QtGui.QMessageBox.Yes) == QtGui.QMessageBox.Yes:
-                book = Book.populate(name=text, publisher=u'')
-                self.manager.save_object(book, False)
-            else:
-                return
-        if self.saveSong():
             Receiver.send_message(u'songs_load_list')
             self.close()
 
     def saveSong(self, preview=False):
         """
         Get all the data from the widgets on the form, and then save it to the
-        database.
+        database.  The form has been validated and all reference items
+        (Authors, Books and Topics) have been saved before this function is
+        called.
 
         ``preview``
             Should be ``True`` if the song is also previewed (boolean).
         """
+        # The Song() assignment.  No database calls should be made while a
+        # Song() is in a partially complete state.
+        if not self.song:
+            self.song = Song()
         self.song.title = unicode(self.titleEdit.text())
         self.song.alternate_title = unicode(self.alternativeEdit.text())
         self.song.copyright = unicode(self.copyrightEdit.text())
@@ -703,15 +709,13 @@ class EditSongForm(QtGui.QDialog, Ui_EditSongDialog):
             self.song.theme_name = theme_name
         else:
             self.song.theme_name = None
-        if self._validate_song():
         self.processLyrics()
         self.processTitle()
         self.song.authors = []
         for row in range(self.authorsListView.count()):
             item = self.authorsListView.item(row)
             authorId = (item.data(QtCore.Qt.UserRole)).toInt()[0]
-                self.song.authors.append(self.manager.get_object(Author,
+            self.song.authors.append(self.manager.get_object(Author, authorId))
-                    authorId))
         self.song.topics = []
         for row in range(self.topicsListView.count()):
             item = self.topicsListView.item(row)
@@ -720,10 +724,12 @@ class EditSongForm(QtGui.QDialog, Ui_EditSongDialog):
         self.manager.save_object(self.song)
         if not preview:
             self.song = None
-            return True
-        return False
 
     def processLyrics(self):
+        """
+        Process the lyric data entered by the user into the OpenLP XML format.
+        """
+        # This method must only be run after the self.song = Song() assignment.
         log.debug(u'processLyrics')
         try:
             sxml = SongXML()
@@ -749,6 +755,11 @@ class EditSongForm(QtGui.QDialog, Ui_EditSongDialog):
                 sxml.dump_xml())
 
     def processTitle(self):
+        """
+        Process the song title entered by the user to remove stray punctuation
+        characters.
+        """
+        # This method must only be run after the self.song = Song() assignment.
         log.debug(u'processTitle')
         self.song.search_title = re.sub(r'[\'"`,;:(){}?]+', u'',
             unicode(self.song.search_title)).lower()

openlp/plugins/songs/lib/db.py

@@ -72,7 +72,7 @@ def init_schema(url):
     ``url``
         The database to setup
     """
-    session, metadata = init_db(url, False)
+    session, metadata = init_db(url)
 
     # Definition of the "authors" table
     authors_table = Table(u'authors', metadata,