← Back to team overview

openlp-core team mailing list archive

[Merge] lp:~sam92/openlp/multiple-songbooks into lp:openlp

 

Samuel Mehrbrodt has proposed merging lp:~sam92/openlp/multiple-songbooks into lp:openlp.

Commit message:
Add support for multiple songbooks

* Migrate DB schema
* Add listbox to song edit dialog and enlarge that dialog a bit
* Rename "Song Book" to "Songbook"
* In the search results, display one row for each songbook entry (the same is done when multiple authors are available)

Requested reviews:
  Raoul Snyman (raoul-snyman)
  Tomas Groth (tomasgroth)

For more details, see:
https://code.launchpad.net/~sam92/openlp/multiple-songbooks/+merge/282000

lp:~sam92/openlp/multiple-songbooks (revision 2602)
[SUCCESS] https://ci.openlp.io/job/Branch-01-Pull/1216/
[FAILURE] https://ci.openlp.io/job/Branch-02-Functional-Tests/1142/
Stopping after failure

The test failure is unrelated.
-- 
Your team OpenLP Core is subscribed to branch lp:openlp.
=== modified file 'openlp/core/common/settings.py'
--- openlp/core/common/settings.py	2015-12-31 22:46:06 +0000
+++ openlp/core/common/settings.py	2016-01-08 14:11:31 +0000
@@ -252,7 +252,8 @@
             'shortcuts/blankScreen': [QtGui.QKeySequence(QtCore.Qt.Key_Period)],
             'shortcuts/collapse': [QtGui.QKeySequence(QtCore.Qt.Key_Minus)],
             'shortcuts/desktopScreen': [QtGui.QKeySequence(QtCore.Qt.Key_D)],
-            'shortcuts/delete': [QtGui.QKeySequence(QtGui.QKeySequence.Delete), QtGui.QKeySequence(QtCore.Qt.Key_Delete)],
+            'shortcuts/delete': [QtGui.QKeySequence(QtGui.QKeySequence.Delete),
+                                 QtGui.QKeySequence(QtCore.Qt.Key_Delete)],
             'shortcuts/down': [QtGui.QKeySequence(QtCore.Qt.Key_Down)],
             'shortcuts/editSong': [],
             'shortcuts/escapeItem': [QtGui.QKeySequence(QtCore.Qt.Key_Escape)],
@@ -329,7 +330,8 @@
             'shortcuts/moveBottom': [QtGui.QKeySequence(QtCore.Qt.Key_End)],
             'shortcuts/moveDown': [QtGui.QKeySequence(QtCore.Qt.Key_PageDown)],
             'shortcuts/nextTrackItem': [],
-            'shortcuts/nextItem_live': [QtGui.QKeySequence(QtCore.Qt.Key_Down), QtGui.QKeySequence(QtCore.Qt.Key_PageDown)],
+            'shortcuts/nextItem_live': [QtGui.QKeySequence(QtCore.Qt.Key_Down),
+                                        QtGui.QKeySequence(QtCore.Qt.Key_PageDown)],
             'shortcuts/nextItem_preview': [QtGui.QKeySequence(QtCore.Qt.Key_Down),
                                            QtGui.QKeySequence(QtCore.Qt.Key_PageDown)],
             'shortcuts/nextService': [QtGui.QKeySequence(QtCore.Qt.Key_Right)],
@@ -339,7 +341,8 @@
                                          QtGui.QKeySequence(QtCore.Qt.ALT + QtCore.Qt.Key_F1)],
             'shortcuts/openService': [],
             'shortcuts/saveService': [],
-            'shortcuts/previousItem_live': [QtGui.QKeySequence(QtCore.Qt.Key_Up), QtGui.QKeySequence(QtCore.Qt.Key_PageUp)],
+            'shortcuts/previousItem_live': [QtGui.QKeySequence(QtCore.Qt.Key_Up),
+                                            QtGui.QKeySequence(QtCore.Qt.Key_PageUp)],
             'shortcuts/playbackPause': [],
             'shortcuts/playbackPlay': [],
             'shortcuts/playbackStop': [],

=== added file 'openlp/core/utils/db.py'
--- openlp/core/utils/db.py	1970-01-01 00:00:00 +0000
+++ openlp/core/utils/db.py	2016-01-08 14:11:31 +0000
@@ -0,0 +1,71 @@
+# -*- coding: utf-8 -*-
+# vim: autoindent shiftwidth=4 expandtab textwidth=120 tabstop=4 softtabstop=4
+
+###############################################################################
+# OpenLP - Open Source Lyrics Projection                                      #
+# --------------------------------------------------------------------------- #
+# Copyright (c) 2008-2016 OpenLP Developers                                   #
+# --------------------------------------------------------------------------- #
+# This program is free software; you can redistribute it and/or modify it     #
+# under the terms of the GNU General Public License as published by the Free  #
+# Software Foundation; version 2 of the License.                              #
+#                                                                             #
+# This program is distributed in the hope that it will be useful, but WITHOUT #
+# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or       #
+# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for    #
+# more details.                                                               #
+#                                                                             #
+# You should have received a copy of the GNU General Public License along     #
+# with this program; if not, write to the Free Software Foundation, Inc., 59  #
+# Temple Place, Suite 330, Boston, MA 02111-1307 USA                          #
+###############################################################################
+"""
+The :mod:`db` module provides helper functions for database related methods.
+"""
+import sqlalchemy
+import logging
+
+from copy import deepcopy
+
+log = logging.getLogger(__name__)
+
+
+def drop_column(op, tablename, columnname):
+    drop_columns(op, tablename, [columnname])
+
+
+def drop_columns(op, tablename, columns):
+    """
+    Column dropping functionality for SQLite, as there is no DROP COLUMN support in SQLite
+
+    From https://github.com/klugjohannes/alembic-sqlite
+    """
+
+    # get the db engine and reflect database tables
+    engine = op.get_bind()
+    meta = sqlalchemy.MetaData(bind=engine)
+    meta.reflect()
+
+    # create a select statement from the old table
+    old_table = meta.tables[tablename]
+    select = sqlalchemy.sql.select([c for c in old_table.c if c.name not in columns])
+
+    # get remaining columns without table attribute attached
+    remaining_columns = [deepcopy(c) for c in old_table.columns if c.name not in columns]
+    for column in remaining_columns:
+        column.table = None
+
+    # create a temporary new table
+    new_tablename = '{0}_new'.format(tablename)
+    op.create_table(new_tablename, *remaining_columns)
+    meta.reflect()
+    new_table = meta.tables[new_tablename]
+
+    # copy data from old table
+    insert = sqlalchemy.sql.insert(new_table).from_select([c.name for c in remaining_columns], select)
+    engine.execute(insert)
+
+    # drop the old table and rename the new table to take the old tables
+    # position
+    op.drop_table(tablename)
+    op.rename_table(new_tablename, tablename)

=== modified file 'openlp/plugins/songs/forms/editsongdialog.py'
--- openlp/plugins/songs/forms/editsongdialog.py	2015-12-31 22:46:06 +0000
+++ openlp/plugins/songs/forms/editsongdialog.py	2016-01-08 14:11:31 +0000
@@ -37,7 +37,7 @@
     def setupUi(self, edit_song_dialog):
         edit_song_dialog.setObjectName('edit_song_dialog')
         edit_song_dialog.setWindowIcon(build_icon(u':/icon/openlp-logo.svg'))
-        edit_song_dialog.resize(650, 400)
+        edit_song_dialog.resize(900, 600)
         edit_song_dialog.setModal(True)
         self.dialog_layout = QtWidgets.QVBoxLayout(edit_song_dialog)
         self.dialog_layout.setSpacing(8)
@@ -173,22 +173,33 @@
         self.topic_remove_layout.addWidget(self.topic_remove_button)
         self.topics_layout.addLayout(self.topic_remove_layout)
         self.authors_right_layout.addWidget(self.topics_group_box)
-        self.song_book_group_box = QtWidgets.QGroupBox(self.authors_tab)
-        self.song_book_group_box.setObjectName('song_book_group_box')
-        self.song_book_layout = QtWidgets.QFormLayout(self.song_book_group_box)
-        self.song_book_layout.setObjectName('song_book_layout')
-        self.song_book_name_label = QtWidgets.QLabel(self.song_book_group_box)
-        self.song_book_name_label.setObjectName('song_book_name_label')
-        self.song_book_combo_box = create_combo_box(self.song_book_group_box, 'song_book_combo_box')
-        self.song_book_name_label.setBuddy(self.song_book_combo_box)
-        self.song_book_layout.addRow(self.song_book_name_label, self.song_book_combo_box)
-        self.song_book_number_label = QtWidgets.QLabel(self.song_book_group_box)
-        self.song_book_number_label.setObjectName('song_book_number_label')
-        self.song_book_number_edit = QtWidgets.QLineEdit(self.song_book_group_box)
-        self.song_book_number_edit.setObjectName('song_book_number_edit')
-        self.song_book_number_label.setBuddy(self.song_book_number_edit)
-        self.song_book_layout.addRow(self.song_book_number_label, self.song_book_number_edit)
-        self.authors_right_layout.addWidget(self.song_book_group_box)
+        self.songbook_group_box = QtWidgets.QGroupBox(self.authors_tab)
+        self.songbook_group_box.setObjectName('songbook_group_box')
+        self.songbooks_layout = QtWidgets.QVBoxLayout(self.songbook_group_box)
+        self.songbooks_layout.setObjectName('songbooks_layout')
+        self.songbook_add_layout = QtWidgets.QHBoxLayout()
+        self.songbook_add_layout.setObjectName('songbook_add_layout')
+        self.songbooks_combo_box = create_combo_box(self.songbook_group_box, 'songbooks_combo_box')
+        self.songbook_add_layout.addWidget(self.songbooks_combo_box)
+        self.songbook_entry_edit = QtWidgets.QLineEdit(self.songbook_group_box)
+        self.songbook_entry_edit.setMaximumWidth(100)
+        self.songbook_add_layout.addWidget(self.songbook_entry_edit)
+        self.songbook_add_button = QtWidgets.QPushButton(self.songbook_group_box)
+        self.songbook_add_button.setObjectName('songbook_add_button')
+        self.songbook_add_layout.addWidget(self.songbook_add_button)
+        self.songbooks_layout.addLayout(self.songbook_add_layout)
+        self.songbooks_list_view = QtWidgets.QListWidget(self.songbook_group_box)
+        self.songbooks_list_view.setAlternatingRowColors(True)
+        self.songbooks_list_view.setObjectName('songbooks_list_view')
+        self.songbooks_layout.addWidget(self.songbooks_list_view)
+        self.songbook_remove_layout = QtWidgets.QHBoxLayout()
+        self.songbook_remove_layout.setObjectName('songbook_remove_layout')
+        self.songbook_remove_layout.addStretch()
+        self.songbook_remove_button = QtWidgets.QPushButton(self.songbook_group_box)
+        self.songbook_remove_button.setObjectName('songbook_remove_button')
+        self.songbook_remove_layout.addWidget(self.songbook_remove_button)
+        self.songbooks_layout.addLayout(self.songbook_remove_layout)
+        self.authors_right_layout.addWidget(self.songbook_group_box)
         self.authors_tab_layout.addLayout(self.authors_right_layout)
         self.song_tab_widget.addTab(self.authors_tab, '')
         # theme tab
@@ -303,15 +314,15 @@
         self.author_add_button.setText(translate('SongsPlugin.EditSongForm', '&Add to Song'))
         self.author_edit_button.setText(translate('SongsPlugin.EditSongForm', '&Edit Author Type'))
         self.author_remove_button.setText(translate('SongsPlugin.EditSongForm', '&Remove'))
-        self.maintenance_button.setText(translate('SongsPlugin.EditSongForm', '&Manage Authors, Topics, Song Books'))
-        self.topics_group_box.setTitle(SongStrings.Topic)
+        self.maintenance_button.setText(translate('SongsPlugin.EditSongForm', '&Manage Authors, Topics, Songbooks'))
+        self.topics_group_box.setTitle(SongStrings.Topics)
         self.topic_add_button.setText(translate('SongsPlugin.EditSongForm', 'A&dd to Song'))
         self.topic_remove_button.setText(translate('SongsPlugin.EditSongForm', 'R&emove'))
-        self.song_book_group_box.setTitle(SongStrings.SongBook)
-        self.song_book_name_label.setText(translate('SongsPlugin.EditSongForm', 'Book:'))
-        self.song_book_number_label.setText(translate('SongsPlugin.EditSongForm', 'Number:'))
+        self.songbook_group_box.setTitle(SongStrings.SongBooks)
+        self.songbook_add_button.setText(translate('SongsPlugin.EditSongForm', 'Add &to Song'))
+        self.songbook_remove_button.setText(translate('SongsPlugin.EditSongForm', 'Re&move'))
         self.song_tab_widget.setTabText(self.song_tab_widget.indexOf(self.authors_tab),
-                                        translate('SongsPlugin.EditSongForm', 'Authors, Topics && Song Book'))
+                                        translate('SongsPlugin.EditSongForm', 'Authors, Topics && Songbooks'))
         self.theme_group_box.setTitle(UiStrings().Theme)
         self.theme_add_button.setText(translate('SongsPlugin.EditSongForm', 'New &Theme'))
         self.rights_group_box.setTitle(translate('SongsPlugin.EditSongForm', 'Copyright Information'))

=== modified file 'openlp/plugins/songs/forms/editsongform.py'
--- openlp/plugins/songs/forms/editsongform.py	2015-12-31 22:46:06 +0000
+++ openlp/plugins/songs/forms/editsongform.py	2016-01-08 14:11:31 +0000
@@ -35,7 +35,7 @@
 from openlp.core.lib import FileDialog, PluginStatus, MediaType, create_separated_list
 from openlp.core.lib.ui import set_case_insensitive_completer, critical_error_message_box, find_and_set_in_combo_box
 from openlp.plugins.songs.lib import VerseType, clean_song
-from openlp.plugins.songs.lib.db import Book, Song, Author, AuthorType, Topic, MediaFile
+from openlp.plugins.songs.lib.db import Book, Song, Author, AuthorType, Topic, MediaFile, SongBookEntry
 from openlp.plugins.songs.lib.ui import SongStrings
 from openlp.plugins.songs.lib.openlyricsxml import SongXML
 from openlp.plugins.songs.forms.editsongdialog import Ui_EditSongDialog
@@ -69,6 +69,9 @@
         self.topic_add_button.clicked.connect(self.on_topic_add_button_clicked)
         self.topic_remove_button.clicked.connect(self.on_topic_remove_button_clicked)
         self.topics_list_view.itemClicked.connect(self.on_topic_list_view_clicked)
+        self.songbook_add_button.clicked.connect(self.on_songbook_add_button_clicked)
+        self.songbook_remove_button.clicked.connect(self.on_songbook_remove_button_clicked)
+        self.songbooks_list_view.itemClicked.connect(self.on_songbook_list_view_clicked)
         self.copyright_insert_button.clicked.connect(self.on_copyright_insert_button_triggered)
         self.verse_add_button.clicked.connect(self.on_verse_add_button_clicked)
         self.verse_list_widget.doubleClicked.connect(self.on_verse_edit_button_clicked)
@@ -125,6 +128,11 @@
         author_item.setData(QtCore.Qt.UserRole, (author.id, author_type))
         self.authors_list_view.addItem(author_item)
 
+    def add_songbookentry_to_list(self, songbook_id, songbook_name, entry):
+        songbookentry_item = QtWidgets.QListWidgetItem(SongBookEntry.get_display_name(songbook_name, entry))
+        songbookentry_item.setData(QtCore.Qt.UserRole, (songbook_id, entry))
+        self.songbooks_list_view.addItem(songbookentry_item)
+
     def _extract_verse_order(self, verse_order):
         """
         Split out the verse order
@@ -219,17 +227,6 @@
             result = self._validate_verse_list(self.verse_order_edit.text(), self.verse_list_widget.rowCount())
             if not result:
                 return False
-        text = self.song_book_combo_box.currentText()
-        if self.song_book_combo_box.findText(text, QtCore.Qt.MatchExactly) < 0:
-            if QtWidgets.QMessageBox.question(
-                    self, translate('SongsPlugin.EditSongForm', 'Add Book'),
-                    translate('SongsPlugin.EditSongForm', 'This song book does not exist, do you want to add it?'),
-                    QtWidgets.QMessageBox.Yes | QtWidgets.QMessageBox.No,
-                    QtWidgets.QMessageBox.Yes) == QtWidgets.QMessageBox.Yes:
-                book = Book.populate(name=text, publisher='')
-                self.manager.save_object(book)
-            else:
-                return False
         # Validate tags (lp#1199639)
         misplaced_tags = []
         verse_tags = []
@@ -327,6 +324,9 @@
             if self.topics_combo_box.hasFocus() and self.topics_combo_box.currentText():
                 self.on_topic_add_button_clicked()
                 return
+            if self.songbooks_combo_box.hasFocus() and self.songbooks_combo_box.currentText():
+                self.on_songbook_add_button_clicked()
+                return
         QtWidgets.QDialog.keyPressEvent(self, event)
 
     def initialise(self):
@@ -367,12 +367,12 @@
         self.topics = []
         self._load_objects(Topic, self.topics_combo_box, self.topics)
 
-    def load_books(self):
-        """
-        Load the song books into the combobox
-        """
-        self.books = []
-        self._load_objects(Book, self.song_book_combo_box, self.books)
+    def load_songbooks(self):
+        """
+        Load the Songbooks into the combobox
+        """
+        self.songbooks = []
+        self._load_objects(Book, self.songbooks_combo_box, self.songbooks)
 
     def load_themes(self, theme_list):
         """
@@ -413,12 +413,13 @@
         self.verse_list_widget.setRowCount(0)
         self.authors_list_view.clear()
         self.topics_list_view.clear()
+        self.songbooks_list_view.clear()
+        self.songbook_entry_edit.clear()
         self.audio_list_widget.clear()
         self.title_edit.setFocus()
-        self.song_book_number_edit.clear()
         self.load_authors()
         self.load_topics()
-        self.load_books()
+        self.load_songbooks()
         self.load_media_files()
         self.theme_combo_box.setEditText('')
         self.theme_combo_box.setCurrentIndex(0)
@@ -437,18 +438,11 @@
         self.song_tab_widget.setCurrentIndex(0)
         self.load_authors()
         self.load_topics()
-        self.load_books()
+        self.load_songbooks()
         self.load_media_files()
         self.song = self.manager.get_object(Song, song_id)
         self.title_edit.setText(self.song.title)
-        self.alternative_edit.setText(
-            self.song.alternate_title if self.song.alternate_title else '')
-        if self.song.song_book_id != 0:
-            book_name = self.manager.get_object(Book, self.song.song_book_id)
-            find_and_set_in_combo_box(self.song_book_combo_box, str(book_name.name))
-        else:
-            self.song_book_combo_box.setEditText('')
-            self.song_book_combo_box.setCurrentIndex(0)
+        self.alternative_edit.setText(self.song.alternate_title if self.song.alternate_title else '')
         if self.song.theme_name:
             find_and_set_in_combo_box(self.theme_combo_box, str(self.song.theme_name))
         else:
@@ -458,7 +452,6 @@
         self.copyright_edit.setText(self.song.copyright if self.song.copyright else '')
         self.comments_edit.setPlainText(self.song.comments if self.song.comments else '')
         self.ccli_number_edit.setText(self.song.ccli_number if self.song.ccli_number else '')
-        self.song_book_number_edit.setText(self.song.song_number if self.song.song_number else '')
         # lazy xml migration for now
         self.verse_list_widget.clear()
         self.verse_list_widget.setRowCount(0)
@@ -520,6 +513,9 @@
             topic_name = QtWidgets.QListWidgetItem(str(topic.name))
             topic_name.setData(QtCore.Qt.UserRole, topic.id)
             self.topics_list_view.addItem(topic_name)
+        self.songbooks_list_view.clear()
+        for songbookentry in self.song.songbookentries:
+            self.add_songbookentry_to_list(songbookentry.songbook.id, songbookentry.songbook.name, songbookentry.entry)
         self.audio_list_widget.clear()
         for media in self.song.media_files:
             media_file = QtWidgets.QListWidgetItem(os.path.split(media.file_name)[1])
@@ -678,6 +674,48 @@
         row = self.topics_list_view.row(item)
         self.topics_list_view.takeItem(row)
 
+    def on_songbook_add_button_clicked(self):
+        item = int(self.songbooks_combo_box.currentIndex())
+        text = self.songbooks_combo_box.currentText()
+        if item == 0 and text:
+            if QtWidgets.QMessageBox.question(
+                    self, translate('SongsPlugin.EditSongForm', 'Add Songbook'),
+                    translate('SongsPlugin.EditSongForm', 'This Songbook does not exist, do you want to add it?'),
+                    QtWidgets.QMessageBox.Yes | QtWidgets.QMessageBox.No,
+                    QtWidgets.QMessageBox.Yes) == QtWidgets.QMessageBox.Yes:
+                songbook = Book.populate(name=text)
+                self.manager.save_object(songbook)
+                self.add_songbookentry_to_list(songbook.id, songbook.name, self.songbook_entry_edit.text())
+                self.load_songbooks()
+                self.songbooks_combo_box.setCurrentIndex(0)
+                self.songbook_entry_edit.clear()
+            else:
+                return
+        elif item > 0:
+            item_id = (self.songbooks_combo_box.itemData(item))
+            songbook = self.manager.get_object(Book, item_id)
+            if self.songbooks_list_view.findItems(str(songbook.name), QtCore.Qt.MatchExactly):
+                critical_error_message_box(
+                    message=translate('SongsPlugin.EditSongForm', 'This Songbook is already in the list.'))
+            else:
+                self.add_songbookentry_to_list(songbook.id, songbook.name, self.songbook_entry_edit.text())
+            self.songbooks_combo_box.setCurrentIndex(0)
+            self.songbook_entry_edit.clear()
+        else:
+            QtWidgets.QMessageBox.warning(
+                self, UiStrings().NISs,
+                translate('SongsPlugin.EditSongForm', 'You have not selected a valid Songbook. Either select a '
+                          'Songbook from the list, or type in a new Songbook and click the "Add to Song" '
+                          'button to add the new Songbook.'))
+
+    def on_songbook_list_view_clicked(self):
+        self.songbook_remove_button.setEnabled(True)
+
+    def on_songbook_remove_button_clicked(self):
+        self.songbook_remove_button.setEnabled(False)
+        row = self.songbooks_list_view.row(self.songbooks_list_view.currentItem())
+        self.songbooks_list_view.takeItem(row)
+
     def on_verse_list_view_clicked(self):
         self.verse_edit_button.setEnabled(True)
         self.verse_delete_button.setEnabled(True)
@@ -838,17 +876,10 @@
         """
         Maintenance button pressed
         """
-        temp_song_book = None
-        item = int(self.song_book_combo_box.currentIndex())
-        text = self.song_book_combo_box.currentText()
-        if item == 0 and text:
-            temp_song_book = text
         self.media_item.song_maintenance_form.exec(True)
         self.load_authors()
-        self.load_books()
+        self.load_songbooks()
         self.load_topics()
-        if temp_song_book:
-            self.song_book_combo_box.setEditText(temp_song_book)
 
     def on_preview(self, button):
         """
@@ -928,7 +959,7 @@
         log.debug('SongEditForm.clearCaches')
         self.authors = []
         self.themes = []
-        self.books = []
+        self.songbooks = []
         self.topics = []
 
     def reject(self):
@@ -977,12 +1008,6 @@
             order.append('%s%s' % (verse_tag, verse_num))
         self.song.verse_order = ' '.join(order)
         self.song.ccli_number = self.ccli_number_edit.text()
-        self.song.song_number = self.song_book_number_edit.text()
-        book_name = self.song_book_combo_box.currentText()
-        if book_name:
-            self.song.book = self.manager.get_object_filtered(Book, Book.name == book_name)
-        else:
-            self.song.book = None
         theme_name = self.theme_combo_box.currentText()
         if theme_name:
             self.song.theme_name = theme_name
@@ -1001,6 +1026,13 @@
             topic = self.manager.get_object(Topic, topic_id)
             if topic is not None:
                 self.song.topics.append(topic)
+        self.song.songbookentries = []
+        for row in range(self.songbooks_list_view.count()):
+            item = self.songbooks_list_view.item(row)
+            songbook_id = item.data(QtCore.Qt.UserRole)[0]
+            songbook = self.manager.get_object(Book, songbook_id)
+            entry = item.data(QtCore.Qt.UserRole)[1]
+            self.song.add_songbookentry(songbook, entry)
         # Save the song here because we need a valid id for the audio files.
         clean_song(self.manager, self.song)
         self.manager.save_object(self.song)

=== modified file 'openlp/plugins/songs/forms/songbookdialog.py'
--- openlp/plugins/songs/forms/songbookdialog.py	2015-12-31 22:46:06 +0000
+++ openlp/plugins/songs/forms/songbookdialog.py	2016-01-08 14:11:31 +0000
@@ -28,7 +28,7 @@
 
 class Ui_SongBookDialog(object):
     """
-    The user interface for the song book dialog.
+    The user interface for the Songbook dialog.
     """
     def setupUi(self, song_book_dialog):
         """
@@ -63,6 +63,6 @@
         """
         Translate the UI on the fly.
         """
-        song_book_dialog.setWindowTitle(translate('SongsPlugin.SongBookForm', 'Song Book Maintenance'))
+        song_book_dialog.setWindowTitle(translate('SongsPlugin.SongBookForm', 'Songbook Maintenance'))
         self.name_label.setText(translate('SongsPlugin.SongBookForm', '&Name:'))
         self.publisher_label.setText(translate('SongsPlugin.SongBookForm', '&Publisher:'))

=== modified file 'openlp/plugins/songs/lib/db.py'
--- openlp/plugins/songs/lib/db.py	2015-12-31 22:46:06 +0000
+++ openlp/plugins/songs/lib/db.py	2016-01-08 14:11:31 +0000
@@ -160,6 +160,36 @@
                 self.authors_songs.remove(author_song)
                 return
 
+    def add_songbookentry(self, songbook, entry):
+        """
+        Add a Songbook Entry to the song if it not yet exists
+
+        :param songbook_name: Name of the Songbook.
+        :param entry: Entry in the Songbook (usually a number)
+        """
+        for songbookentry in self.songbookentries:
+            if songbookentry.songbook.name == songbook.name and songbookentry.entry == entry:
+                return
+
+        new_songbookentry = SongBookEntry()
+        new_songbookentry.songbook = songbook
+        new_songbookentry.entry = entry
+        self.songbookentries.append(new_songbookentry)
+
+
+class SongBookEntry(BaseModel):
+    """
+    SongBookEntry model
+    """
+    def __repr__(self):
+        return SongBookEntry.get_display_name(self.songbook.name, self.entry)
+
+    @staticmethod
+    def get_display_name(songbook_name, entry):
+        if entry:
+            return "%s #%s" % (songbook_name, entry)
+        return songbook_name
+
 
 class Topic(BaseModel):
     """
@@ -182,6 +212,7 @@
         * media_files_songs
         * song_books
         * songs
+        * songs_songbooks
         * songs_topics
         * topics
 
@@ -222,7 +253,6 @@
         The *songs* table has the following columns:
 
         * id
-        * song_book_id
         * title
         * alternate_title
         * lyrics
@@ -230,11 +260,17 @@
         * copyright
         * comments
         * ccli_number
-        * song_number
         * theme_name
         * search_title
         * search_lyrics
 
+    **songs_songsbooks Table**
+        This is a mapping table between the *songs* and the *song_books* tables. It has the following columns:
+
+        * songbook_id
+        * song_id
+        * entry  # The song number, like 120 or 550A
+
     **songs_topics Table**
         This is a bridging table between the *songs* and *topics* tables, which
         serves to create a many-to-many relationship between the two tables. It
@@ -284,7 +320,6 @@
     songs_table = Table(
         'songs', metadata,
         Column('id', types.Integer(), primary_key=True),
-        Column('song_book_id', types.Integer(), ForeignKey('song_books.id'), default=None),
         Column('title', types.Unicode(255), nullable=False),
         Column('alternate_title', types.Unicode(255)),
         Column('lyrics', types.UnicodeText, nullable=False),
@@ -292,7 +327,6 @@
         Column('copyright', types.Unicode(255)),
         Column('comments', types.UnicodeText),
         Column('ccli_number', types.Unicode(64)),
-        Column('song_number', types.Unicode(64)),
         Column('theme_name', types.Unicode(128)),
         Column('search_title', types.Unicode(255), index=True, nullable=False),
         Column('search_lyrics', types.UnicodeText, nullable=False),
@@ -316,6 +350,14 @@
         Column('author_type', types.Unicode(255), primary_key=True, nullable=False, server_default=text('""'))
     )
 
+    # Definition of the "songs_songbooks" table
+    songs_songbooks_table = Table(
+        'songs_songbooks', metadata,
+        Column('songbook_id', types.Integer(), ForeignKey('song_books.id'), primary_key=True),
+        Column('song_id', types.Integer(), ForeignKey('songs.id'), primary_key=True),
+        Column('entry', types.Unicode(255), primary_key=True, nullable=False)
+    )
+
     # Definition of the "songs_topics" table
     songs_topics_table = Table(
         'songs_topics', metadata,
@@ -329,6 +371,9 @@
     mapper(AuthorSong, authors_songs_table, properties={
         'author': relation(Author)
     })
+    mapper(SongBookEntry, songs_songbooks_table, properties={
+        'songbook': relation(Book)
+    })
     mapper(Book, song_books_table)
     mapper(MediaFile, media_files_table)
     mapper(Song, songs_table, properties={
@@ -337,8 +382,8 @@
         'authors_songs': relation(AuthorSong, cascade="all, delete-orphan"),
         # Use lazy='joined' to always load authors when the song is fetched from the database (bug 1366198)
         'authors': relation(Author, secondary=authors_songs_table, viewonly=True, lazy='joined'),
-        'book': relation(Book, backref='songs'),
         'media_files': relation(MediaFile, backref='songs', order_by=media_files_table.c.weight),
+        'songbookentries': relation(SongBookEntry, backref='song', cascade="all, delete-orphan"),
         'topics': relation(Topic, backref='songs', secondary=songs_topics_table)
     })
     mapper(Topic, topics_table)

=== modified file 'openlp/plugins/songs/lib/mediaitem.py'
--- openlp/plugins/songs/lib/mediaitem.py	2015-12-31 22:46:06 +0000
+++ openlp/plugins/songs/lib/mediaitem.py	2016-01-08 14:11:31 +0000
@@ -37,7 +37,7 @@
 from openlp.plugins.songs.forms.songimportform import SongImportForm
 from openlp.plugins.songs.forms.songexportform import SongExportForm
 from openlp.plugins.songs.lib import VerseType, clean_string, delete_song
-from openlp.plugins.songs.lib.db import Author, AuthorType, Song, Book, MediaFile
+from openlp.plugins.songs.lib.db import Author, AuthorType, Song, Book, MediaFile, SongBookEntry
 from openlp.plugins.songs.lib.ui import SongStrings
 from openlp.plugins.songs.lib.openlyricsxml import OpenLyrics, SongXML
 
@@ -151,7 +151,7 @@
             (SongSearch.Authors, ':/songs/song_search_author.png', SongStrings.Authors,
                 translate('SongsPlugin.MediaItem', 'Search Authors...')),
             (SongSearch.Books, ':/songs/song_book_edit.png', SongStrings.SongBooks,
-                translate('SongsPlugin.MediaItem', 'Search Song Books...')),
+                translate('SongsPlugin.MediaItem', 'Search Songbooks...')),
             (SongSearch.Themes, ':/slides/slide_theme.png', UiStrings().Themes, UiStrings().SearchThemes)
         ])
         self.search_text_edit.set_current_search_type(Settings().value('%s/last search type' % self.settings_section))
@@ -184,17 +184,8 @@
                 Author, Author.display_name.like(search_string), Author.display_name.asc())
             self.display_results_author(search_results)
         elif search_type == SongSearch.Books:
-            log.debug('Books Search')
-            search_string = '%' + search_keywords + '%'
-            search_results = self.plugin.manager.get_all_objects(Book, Book.name.like(search_string), Book.name.asc())
-            song_number = False
-            if not search_results:
-                search_keywords = search_keywords.rpartition(' ')
-                search_string = '%' + search_keywords[0] + '%'
-                search_results = self.plugin.manager.get_all_objects(Book,
-                                                                     Book.name.like(search_string), Book.name.asc())
-                song_number = re.sub(r'[^0-9]', '', search_keywords[2])
-            self.display_results_book(search_results, song_number)
+            log.debug('Songbook Search')
+            self.display_results_book(search_keywords)
         elif search_type == SongSearch.Themes:
             log.debug('Theme Search')
             search_string = '%' + search_keywords + '%'
@@ -254,21 +245,29 @@
                 song_name.setData(QtCore.Qt.UserRole, song.id)
                 self.list_view.addItem(song_name)
 
-    def display_results_book(self, search_results, song_number=False):
+    def display_results_book(self, search_keywords):
         log.debug('display results Book')
         self.list_view.clear()
-        for book in search_results:
-            songs = sorted(book.songs, key=lambda song: int(re.match(r'[0-9]+', '0' + song.song_number).group()))
-            for song in songs:
-                # Do not display temporary songs
-                if song.temporary:
-                    continue
-                if song_number and song_number not in song.song_number:
-                    continue
-                song_detail = '%s - %s (%s)' % (book.name, song.song_number, song.title)
-                song_name = QtWidgets.QListWidgetItem(song_detail)
-                song_name.setData(QtCore.Qt.UserRole, song.id)
-                self.list_view.addItem(song_name)
+
+        search_keywords = search_keywords.rpartition(' ')
+        search_book = search_keywords[0]
+        search_entry = re.sub(r'[^0-9]', '', search_keywords[2])
+
+        songbookentries = (self.plugin.manager.session.query(SongBookEntry)
+                           .join(Book)
+                           .order_by(Book.name)
+                           .order_by(SongBookEntry.entry))
+        for songbookentry in songbookentries:
+            if songbookentry.song.temporary:
+                continue
+            if search_book.lower() not in songbookentry.songbook.name.lower():
+                continue
+            if search_entry not in songbookentry.entry:
+                continue
+            song_detail = '%s #%s: %s' % (songbookentry.songbook.name, songbookentry.entry, songbookentry.song.title)
+            song_name = QtWidgets.QListWidgetItem(song_detail)
+            song_name.setData(QtCore.Qt.UserRole, songbookentry.song.id)
+            self.list_view.addItem(song_name)
 
     def on_clear_text_button_click(self):
         """
@@ -523,8 +522,9 @@
                 item.raw_footer.append("%s %s" % (SongStrings.CopyrightSymbol, song.copyright))
             else:
                 item.raw_footer.append(song.copyright)
-        if self.display_songbook and song.book:
-            item.raw_footer.append("%s #%s" % (song.book.name, song.song_number))
+        if self.display_songbook and song.songbookentries:
+            songbooks = [str(songbookentry) for songbookentry in song.songbookentries]
+            item.raw_footer.append(", ".join(songbooks))
         if Settings().value('core/ccli number'):
             item.raw_footer.append(translate('SongsPlugin.MediaItem',
                                              'CCLI License: ') + Settings().value('core/ccli number'))

=== modified file 'openlp/plugins/songs/lib/openlyricsxml.py'
--- openlp/plugins/songs/lib/openlyricsxml.py	2015-12-31 22:46:06 +0000
+++ openlp/plugins/songs/lib/openlyricsxml.py	2016-01-08 14:11:31 +0000
@@ -266,13 +266,12 @@
                         element.set('type', AuthorType.Music)
                     else:
                         element.set('type', author_song.author_type)
-        book = self.manager.get_object_filtered(Book, Book.id == song.song_book_id)
-        if book is not None:
-            book = book.name
+        if song.songbookentries:
             songbooks = etree.SubElement(properties, 'songbooks')
-            element = self._add_text_to_element('songbook', songbooks, None, book)
-            if song.song_number:
-                element.set('entry', song.song_number)
+            for songbookentry in song.songbookentries:
+                element = self._add_text_to_element('songbook', songbooks, None, songbookentry.songbook.name)
+                if songbookentry.entry:
+                    element.set('entry', songbookentry.entry)
         if song.topics:
             themes = etree.SubElement(properties, 'themes')
             for topic in song.topics:
@@ -744,8 +743,6 @@
         :param properties: The property object (lxml.objectify.ObjectifiedElement).
         :param song: The song object.
         """
-        song.song_book_id = None
-        song.song_number = ''
         if hasattr(properties, 'songbooks'):
             for songbook in properties.songbooks.songbook:
                 book_name = songbook.get('name', '')
@@ -755,10 +752,7 @@
                         # We need to create a book, because it does not exist.
                         book = Book.populate(name=book_name, publisher='')
                         self.manager.save_object(book)
-                    song.song_book_id = book.id
-                    song.song_number = songbook.get('entry', '')
-                    # We only support one song book, so take the first one.
-                    break
+                    song.add_songbookentry(book, songbook.get('entry', ''))
 
     def _process_titles(self, properties, song):
         """

=== modified file 'openlp/plugins/songs/lib/ui.py'
--- openlp/plugins/songs/lib/ui.py	2015-12-31 22:46:06 +0000
+++ openlp/plugins/songs/lib/ui.py	2016-01-08 14:11:31 +0000
@@ -35,8 +35,8 @@
     Authors = translate('OpenLP.Ui', 'Authors', 'Plural')
     AuthorUnknown = translate('OpenLP.Ui', 'Author Unknown')  # Used to populate the database.
     CopyrightSymbol = translate('OpenLP.Ui', '\xa9', 'Copyright symbol.')
-    SongBook = translate('OpenLP.Ui', 'Song Book', 'Singular')
-    SongBooks = translate('OpenLP.Ui', 'Song Books', 'Plural')
+    SongBook = translate('OpenLP.Ui', 'Songbook', 'Singular')
+    SongBooks = translate('OpenLP.Ui', 'Songbooks', 'Plural')
     SongIncomplete = translate('OpenLP.Ui', 'Title and/or verses not found')
     SongMaintenance = translate('OpenLP.Ui', 'Song Maintenance')
     Topic = translate('OpenLP.Ui', 'Topic', 'Singular')

=== modified file 'openlp/plugins/songs/lib/upgrade.py'
--- openlp/plugins/songs/lib/upgrade.py	2015-12-31 22:46:06 +0000
+++ openlp/plugins/songs/lib/upgrade.py	2016-01-08 14:11:31 +0000
@@ -29,10 +29,10 @@
 from sqlalchemy.sql.expression import func, false, null, text
 
 from openlp.core.lib.db import get_upgrade_op
-from openlp.core.common import trace_error_handler
+from openlp.core.utils.db import drop_columns
 
 log = logging.getLogger(__name__)
-__version__ = 4
+__version__ = 5
 
 
 # TODO: When removing an upgrade path the ftw-data needs updating to the minimum supported version
@@ -117,3 +117,34 @@
         op.rename_table('authors_songs_tmp', 'authors_songs')
     else:
         log.warning('Skipping upgrade_4 step of upgrading the song db')
+
+
+def upgrade_5(session, metadata):
+    """
+    Version 5 upgrade.
+
+    This upgrade adds support for multiple songbooks
+    """
+    op = get_upgrade_op(session)
+    songs_table = Table('songs', metadata)
+    if 'song_book_id' in [col.name for col in songs_table.c.values()]:
+        log.warning('Skipping upgrade_5 step of upgrading the song db')
+        return
+
+    # Create the mapping table (songs <-> songbooks)
+    op.create_table('songs_songbooks',
+                    Column('songbook_id', types.Integer(), ForeignKey('song_books.id'), primary_key=True),
+                    Column('song_id', types.Integer(), ForeignKey('songs.id'), primary_key=True),
+                    Column('entry', types.Unicode(255), primary_key=True, nullable=False))
+
+    # Migrate old data
+    op.execute('INSERT INTO songs_songbooks SELECT song_book_id, id, song_number FROM songs\
+                WHERE song_book_id IS NOT NULL AND song_number IS NOT NULL')
+
+    # Drop old columns
+    if metadata.bind.url.get_dialect().name == 'sqlite':
+        drop_columns(op, 'songs', ['song_book_id', 'song_number'])
+    else:
+        op.drop_constraint('songs_ibfk_1', 'songs', 'foreignkey')
+        op.drop_column('songs', 'song_book_id')
+        op.drop_column('songs', 'song_number')

=== added file 'tests/functional/openlp_core_utils/test_db.py'
--- tests/functional/openlp_core_utils/test_db.py	1970-01-01 00:00:00 +0000
+++ tests/functional/openlp_core_utils/test_db.py	2016-01-08 14:11:31 +0000
@@ -0,0 +1,96 @@
+# -*- coding: utf-8 -*-
+# vim: autoindent shiftwidth=4 expandtab textwidth=120 tabstop=4 softtabstop=4
+
+###############################################################################
+# OpenLP - Open Source Lyrics Projection                                      #
+# --------------------------------------------------------------------------- #
+# Copyright (c) 2008-2016 OpenLP Developers                                   #
+# --------------------------------------------------------------------------- #
+# This program is free software; you can redistribute it and/or modify it     #
+# under the terms of the GNU General Public License as published by the Free  #
+# Software Foundation; version 2 of the License.                              #
+#                                                                             #
+# This program is distributed in the hope that it will be useful, but WITHOUT #
+# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or       #
+# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for    #
+# more details.                                                               #
+#                                                                             #
+# You should have received a copy of the GNU General Public License along     #
+# with this program; if not, write to the Free Software Foundation, Inc., 59  #
+# Temple Place, Suite 330, Boston, MA 02111-1307 USA                          #
+###############################################################################
+"""
+Package to test the openlp.core.utils.db package.
+"""
+import os
+import shutil
+import sqlalchemy
+from unittest import TestCase
+from tempfile import mkdtemp
+
+from openlp.core.utils.db import drop_column, drop_columns
+from openlp.core.lib.db import init_db, get_upgrade_op
+from tests.utils.constants import TEST_RESOURCES_PATH
+
+
+class TestUtilsDBFunctions(TestCase):
+
+    def setUp(self):
+        """
+        Create temp folder for keeping db file
+        """
+        self.tmp_folder = mkdtemp()
+
+    def tearDown(self):
+        """
+        Clean up
+        """
+        shutil.rmtree(self.tmp_folder)
+
+    def delete_column_test(self):
+        """
+        Test deleting a single column in a table
+        """
+        # GIVEN: A temporary song db
+        db_path = os.path.join(TEST_RESOURCES_PATH, 'songs', 'songs-1.9.7.sqlite')
+        db_tmp_path = os.path.join(self.tmp_folder, 'songs-1.9.7.sqlite')
+        shutil.copyfile(db_path, db_tmp_path)
+        db_url = 'sqlite:///' + db_tmp_path
+        session, metadata = init_db(db_url)
+        op = get_upgrade_op(session)
+
+        # WHEN: Deleting a columns in a table
+        drop_column(op, 'songs', 'song_book_id')
+
+        # THEN: The column should have been deleted
+        meta = sqlalchemy.MetaData(bind=op.get_bind())
+        meta.reflect()
+        columns = meta.tables['songs'].columns
+
+        for column in columns:
+            if column.name == 'song_book_id':
+                self.fail("The column 'song_book_id' should have been deleted.")
+
+    def delete_columns_test(self):
+        """
+        Test deleting multiple columns in a table
+        """
+        # GIVEN: A temporary song db
+        db_path = os.path.join(TEST_RESOURCES_PATH, 'songs', 'songs-1.9.7.sqlite')
+        db_tmp_path = os.path.join(self.tmp_folder, 'songs-1.9.7.sqlite')
+        shutil.copyfile(db_path, db_tmp_path)
+        db_url = 'sqlite:///' + db_tmp_path
+        session, metadata = init_db(db_url)
+        op = get_upgrade_op(session)
+
+        # WHEN: Deleting a columns in a table
+        drop_columns(op, 'songs', ['song_book_id', 'song_number'])
+
+        # THEN: The columns should have been deleted
+        meta = sqlalchemy.MetaData(bind=op.get_bind())
+        meta.reflect()
+        columns = meta.tables['songs'].columns
+
+        for column in columns:
+            if column.name == 'song_book_id' or column.name == 'song_number':
+                self.fail("The column '%s' should have been deleted." % column.name)

=== modified file 'tests/functional/openlp_plugins/songs/test_db.py'
--- tests/functional/openlp_plugins/songs/test_db.py	2015-12-31 22:46:06 +0000
+++ tests/functional/openlp_plugins/songs/test_db.py	2016-01-08 14:11:31 +0000
@@ -27,7 +27,7 @@
 from unittest import TestCase
 from tempfile import mkdtemp
 
-from openlp.plugins.songs.lib.db import Song, Author, AuthorType
+from openlp.plugins.songs.lib.db import Song, Author, AuthorType, Book
 from openlp.plugins.songs.lib import upgrade
 from openlp.core.lib.db import upgrade_db
 from tests.utils.constants import TEST_RESOURCES_PATH
@@ -179,6 +179,23 @@
         # THEN: It should return the name with the type in brackets
         self.assertEqual("John Doe (Translation)", display_name)
 
+    def test_add_songbooks(self):
+        """
+        Test that adding songbooks to a song works correctly
+        """
+        # GIVEN: A mocked song and songbook
+        song = Song()
+        song.songbookentries = []
+        songbook = Book()
+        songbook.name = "Thy Word"
+
+        # WHEN: We add two songbooks to a Song
+        song.add_songbookentry(songbook, "120")
+        song.add_songbookentry(songbook, "550A")
+
+        # THEN: The song should have two songbook entries
+        self.assertEqual(len(song.songbookentries), 2, 'There should be two Songbook entries.')
+
     def test_upgrade_old_song_db(self):
         """
         Test that we can upgrade an old song db to the current schema
@@ -192,7 +209,7 @@
         # WHEN: upgrading the db
         updated_to_version, latest_version = upgrade_db(db_url, upgrade)
 
-        # Then the song db should have been upgraded to the latest version
+        # THEN: the song db should have been upgraded to the latest version
         self.assertEqual(updated_to_version, latest_version,
                          'The song DB should have been upgrade to the latest version')
 
@@ -209,6 +226,6 @@
         # WHEN: upgrading the db
         updated_to_version, latest_version = upgrade_db(db_url, upgrade)
 
-        # Then the song db should have been upgraded to the latest version without errors
+        # THEN: the song db should have been upgraded to the latest version without errors
         self.assertEqual(updated_to_version, latest_version,
                          'The song DB should have been upgrade to the latest version')

=== modified file 'tests/functional/openlp_plugins/songs/test_mediaitem.py'
--- tests/functional/openlp_plugins/songs/test_mediaitem.py	2015-12-31 22:46:06 +0000
+++ tests/functional/openlp_plugins/songs/test_mediaitem.py	2016-01-08 14:11:31 +0000
@@ -29,7 +29,7 @@
 from openlp.core.common import Registry, Settings
 from openlp.core.lib import ServiceItem
 from openlp.plugins.songs.lib.mediaitem import SongMediaItem
-from openlp.plugins.songs.lib.db import AuthorType
+from openlp.plugins.songs.lib.db import AuthorType, Song
 from tests.functional import patch, MagicMock
 from tests.helpers.testmixin import TestMixin
 
@@ -152,15 +152,21 @@
 
     def build_song_footer_base_songbook_test(self):
         """
-        Test build songs footer with basic song and a songbook
+        Test build songs footer with basic song and multiple songbooks
         """
         # GIVEN: A Song and a Service Item
-        mock_song = MagicMock()
+        mock_song = Song()
         mock_song.title = 'My Song'
         mock_song.copyright = 'My copyright'
-        mock_song.book = MagicMock()
-        mock_song.book.name = "My songbook"
-        mock_song.song_number = 12
+        mock_song.authors_songs = []
+        mock_song.ccli_number = ''
+        book1 = MagicMock()
+        book1.name = "My songbook"
+        book2 = MagicMock()
+        book2.name = "Thy songbook"
+        mock_song.songbookentries = []
+        mock_song.add_songbookentry(book1, '12')
+        mock_song.add_songbookentry(book2, '502A')
         service_item = ServiceItem(None)
 
         # WHEN: I generate the Footer with default settings
@@ -174,7 +180,7 @@
         self.media_item.generate_footer(service_item, mock_song)
 
         # THEN: The songbook should be in the footer
-        self.assertEqual(service_item.raw_footer, ['My Song', 'My copyright', 'My songbook #12'])
+        self.assertEqual(service_item.raw_footer, ['My Song', 'My copyright', 'My songbook #12, Thy songbook #502A'])
 
     def build_song_footer_copyright_enabled_test(self):
         """


Follow ups