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: Tomas Groth (tomasgroth) Raoul Snyman (raoul-snyman) For more details, see: https://code.launchpad.net/~sam92/openlp/multiple-songbooks/+merge/282095 lp:~sam92/openlp/multiple-songbooks (revision 2607) [SUCCESS] https://ci.openlp.io/job/Branch-01-Pull/1226/ [SUCCESS] https://ci.openlp.io/job/Branch-02-Functional-Tests/1151/ [SUCCESS] https://ci.openlp.io/job/Branch-03-Interface-Tests/1090/ [FAILURE] https://ci.openlp.io/job/Branch-04a-Windows_Functional_Tests/928/ Stopping after failure -- Your team OpenLP Core is subscribed to branch lp:openlp.
=== 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-09 16:14:13 +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-09 16:14:13 +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 2016-01-08 17:28:47 +0000 +++ openlp/plugins/songs/forms/editsongform.py 2016-01-09 16:14:13 +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 @@ -70,6 +70,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) @@ -126,6 +129,11 @@ author_item.setData(QtCore.Qt.UserRole, (author.id, author_type)) self.authors_list_view.addItem(author_item) + def add_songbook_entry_to_list(self, songbook_id, songbook_name, entry): + songbook_entry_item = QtWidgets.QListWidgetItem(SongBookEntry.get_display_name(songbook_name, entry)) + songbook_entry_item.setData(QtCore.Qt.UserRole, (songbook_id, entry)) + self.songbooks_list_view.addItem(songbook_entry_item) + def _extract_verse_order(self, verse_order): """ Split out the verse order @@ -220,17 +228,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 = [] @@ -328,6 +325,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): @@ -368,12 +368,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): """ @@ -414,12 +414,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) @@ -438,18 +439,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: @@ -459,7 +453,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) @@ -521,6 +514,10 @@ 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 songbook_entry in self.song.songbook_entries: + self.add_songbook_entry_to_list(songbook_entry.songbook.id, songbook_entry.songbook.name, + songbook_entry.entry) self.audio_list_widget.clear() for media in self.song.media_files: media_file = QtWidgets.QListWidgetItem(os.path.split(media.file_name)[1]) @@ -679,6 +676,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_songbook_entry_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_songbook_entry_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) @@ -841,17 +880,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): """ @@ -931,7 +963,7 @@ log.debug('SongEditForm.clearCaches') self.authors = [] self.themes = [] - self.books = [] + self.songbooks = [] self.topics = [] def reject(self): @@ -980,12 +1012,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 @@ -1004,6 +1030,13 @@ topic = self.manager.get_object(Topic, topic_id) if topic is not None: self.song.topics.append(topic) + self.song.songbook_entries = [] + 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_songbook_entry(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-09 16:14:13 +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-09 16:14:13 +0000 @@ -160,6 +160,36 @@ self.authors_songs.remove(author_song) return + def add_songbook_entry(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 songbook_entry in self.songbook_entries: + if songbook_entry.songbook.name == songbook.name and songbook_entry.entry == entry: + return + + new_songbook_entry = SongBookEntry() + new_songbook_entry.songbook = songbook + new_songbook_entry.entry = entry + self.songbook_entries.append(new_songbook_entry) + + +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), + 'songbook_entries': 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 2016-01-08 17:28:47 +0000 +++ openlp/plugins/songs/lib/mediaitem.py 2016-01-09 16:14:13 +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 @@ -152,7 +152,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)) @@ -185,17 +185,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 + '%' @@ -255,21 +246,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]) + + songbook_entries = (self.plugin.manager.session.query(SongBookEntry) + .join(Book) + .order_by(Book.name) + .order_by(SongBookEntry.entry)) + for songbook_entry in songbook_entries: + if songbook_entry.song.temporary: + continue + if search_book.lower() not in songbook_entry.songbook.name.lower(): + continue + if search_entry not in songbook_entry.entry: + continue + song_detail = '%s #%s: %s' % (songbook_entry.songbook.name, songbook_entry.entry, songbook_entry.song.title) + song_name = QtWidgets.QListWidgetItem(song_detail) + song_name.setData(QtCore.Qt.UserRole, songbook_entry.song.id) + self.list_view.addItem(song_name) def on_clear_text_button_click(self): """ @@ -524,8 +523,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.songbook_entries: + songbooks = [str(songbook_entry) for songbook_entry in song.songbook_entries] + 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-09 16:14:13 +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.songbook_entries: 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 songbook_entry in song.songbook_entries: + element = self._add_text_to_element('songbook', songbooks, None, songbook_entry.songbook.name) + if songbook_entry.entry: + element.set('entry', songbook_entry.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_songbook_entry(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-09 16:14:13 +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-09 16:14:13 +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-09 16:14:13 +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-09 16:14:13 +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.songbook_entries = [] + songbook = Book() + songbook.name = "Thy Word" + + # WHEN: We add two songbooks to a Song + song.add_songbook_entry(songbook, "120") + song.add_songbook_entry(songbook, "550A") + + # THEN: The song should have two songbook entries + self.assertEqual(len(song.songbook_entries), 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 2016-01-05 19:32:12 +0000 +++ tests/functional/openlp_plugins/songs/test_mediaitem.py 2016-01-09 16:14:13 +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,29 +152,36 @@ 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.title = 'My Song' - mock_song.copyright = 'My copyright' - mock_song.book = MagicMock() - mock_song.book.name = "My songbook" - mock_song.song_number = 12 + song = Song() + song.title = 'My Song' + song.copyright = 'My copyright' + song.authors_songs = [] + song.songbook_entries = [] + song.ccli_number = '' + book1 = MagicMock() + book1.name = "My songbook" + book2 = MagicMock() + book2.name = "Thy songbook" + song.songbookentries = [] + song.add_songbook_entry(book1, '12') + song.add_songbook_entry(book2, '502A') service_item = ServiceItem(None) # WHEN: I generate the Footer with default settings - self.media_item.generate_footer(service_item, mock_song) + self.media_item.generate_footer(service_item, song) # THEN: The songbook should not be in the footer self.assertEqual(service_item.raw_footer, ['My Song', 'My copyright']) # WHEN: I activate the "display songbook" option self.media_item.display_songbook = True - self.media_item.generate_footer(service_item, mock_song) + self.media_item.generate_footer(service_item, 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): """
_______________________________________________ Mailing list: https://launchpad.net/~openlp-core Post to : [email protected] Unsubscribe : https://launchpad.net/~openlp-core More help : https://help.launchpad.net/ListHelp
