From 71b34578d71b6c112d57c91da02f7a17a07d599f Mon Sep 17 00:00:00 2001 From: Samuel Mehrbrodt Date: Fri, 28 Mar 2014 17:06:16 +0100 Subject: [PATCH 01/28] Add support for the Author type property --- openlp/core/lib/db.py | 3 +- openlp/plugins/songs/forms/editsongdialog.py | 9 ++++- openlp/plugins/songs/forms/editsongform.py | 40 +++++++++++--------- openlp/plugins/songs/lib/__init__.py | 7 ---- openlp/plugins/songs/lib/db.py | 33 +++++++++++++++- openlp/plugins/songs/lib/mediaitem.py | 32 +++++++++++++--- openlp/plugins/songs/lib/upgrade.py | 11 +++++- 7 files changed, 100 insertions(+), 35 deletions(-) diff --git a/openlp/core/lib/db.py b/openlp/core/lib/db.py index c81cb5a3a..b4e027035 100644 --- a/openlp/core/lib/db.py +++ b/openlp/core/lib/db.py @@ -194,6 +194,7 @@ class Manager(object): db_ver, up_ver = upgrade_db(self.db_url, upgrade_mod) except (SQLAlchemyError, DBAPIError): log.exception('Error loading database: %s', self.db_url) + return if db_ver > up_ver: critical_error_message_box( translate('OpenLP.Manager', 'Database Error'), @@ -215,7 +216,7 @@ class Manager(object): Save an object to the database :param object_instance: The object to save - :param commit: Commit the session with this object + :param commit: Commit the session with this object """ for try_count in range(3): try: diff --git a/openlp/plugins/songs/forms/editsongdialog.py b/openlp/plugins/songs/forms/editsongdialog.py index f2ef5af06..7a491032b 100644 --- a/openlp/plugins/songs/forms/editsongdialog.py +++ b/openlp/plugins/songs/forms/editsongdialog.py @@ -122,6 +122,11 @@ class Ui_EditSongDialog(object): self.author_add_layout.setObjectName('author_add_layout') self.authors_combo_box = create_combo_box(self.authors_group_box, 'authors_combo_box') self.author_add_layout.addWidget(self.authors_combo_box) + self.author_types_combo_box = create_combo_box(self.authors_group_box, 'author_types_combo_box', editable=False) + # Need to give these boxes some min width, else they are too small + self.authors_combo_box.setMinimumWidth(150) + self.author_types_combo_box.setMinimumWidth(80) + self.author_add_layout.addWidget(self.author_types_combo_box) self.author_add_button = QtGui.QPushButton(self.authors_group_box) self.author_add_button.setObjectName('author_add_button') self.author_add_layout.addWidget(self.author_add_button) @@ -330,7 +335,7 @@ class Ui_EditSongDialog(object): translate('SongsPlugin.EditSongForm', 'Warning: You have not entered a verse order.') -def create_combo_box(parent, name): +def create_combo_box(parent, name, editable=True): """ Utility method to generate a standard combo box for this dialog. @@ -340,7 +345,7 @@ def create_combo_box(parent, name): combo_box = QtGui.QComboBox(parent) combo_box.setSizeAdjustPolicy(QtGui.QComboBox.AdjustToMinimumContentsLength) combo_box.setSizePolicy(QtGui.QSizePolicy.Expanding, QtGui.QSizePolicy.Fixed) - combo_box.setEditable(True) + combo_box.setEditable(editable) combo_box.setInsertPolicy(QtGui.QComboBox.NoInsert) combo_box.setObjectName(name) return combo_box diff --git a/openlp/plugins/songs/forms/editsongform.py b/openlp/plugins/songs/forms/editsongform.py index 8b1e3a897..2c651e983 100644 --- a/openlp/plugins/songs/forms/editsongform.py +++ b/openlp/plugins/songs/forms/editsongform.py @@ -42,7 +42,7 @@ from openlp.core.common import Registry, RegistryProperties, AppLocation, UiStri 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, Topic, MediaFile +from openlp.plugins.songs.lib.db import Book, Song, Author, AuthorSong, Topic, MediaFile from openlp.plugins.songs.lib.ui import SongStrings from openlp.plugins.songs.lib.xml import SongXML from openlp.plugins.songs.forms.editsongdialog import Ui_EditSongDialog @@ -122,12 +122,12 @@ class EditSongForm(QtGui.QDialog, Ui_EditSongDialog, RegistryProperties): combo.setItemData(row, obj.id) set_case_insensitive_completer(cache, combo) - def _add_author_to_list(self, author): + def _add_author_to_list(self, author, author_type): """ Add an author to the author list. """ - author_item = QtGui.QListWidgetItem(str(author.display_name)) - author_item.setData(QtCore.Qt.UserRole, author.id) + author_item = QtGui.QListWidgetItem(author.get_display_name(author_type)) + author_item.setData(QtCore.Qt.UserRole, (author.id, author_type)) self.authors_list_view.addItem(author_item) def _extract_verse_order(self, verse_order): @@ -302,6 +302,14 @@ class EditSongForm(QtGui.QDialog, Ui_EditSongDialog, RegistryProperties): self.authors.append(author.display_name) set_case_insensitive_completer(self.authors, self.authors_combo_box) + #Types + self.author_types_combo_box.clear() + self.author_types_combo_box.addItem('') + # Don't iterate over the dictionary to give them this specific order + self.author_types_combo_box.addItem(Author.Types[Author.TYPE_WORDS], Author.TYPE_WORDS) + self.author_types_combo_box.addItem(Author.Types[Author.TYPE_MUSIC], Author.TYPE_MUSIC) + self.author_types_combo_box.addItem(Author.Types[Author.TYPE_TRANSLATION], Author.TYPE_TRANSLATION) + def load_topics(self): """ Load the topics into the combobox. @@ -454,10 +462,8 @@ class EditSongForm(QtGui.QDialog, Ui_EditSongDialog, RegistryProperties): self.tag_rows() # clear the results self.authors_list_view.clear() - for author in self.song.authors: - author_name = QtGui.QListWidgetItem(str(author.display_name)) - author_name.setData(QtCore.Qt.UserRole, author.id) - self.authors_list_view.addItem(author_name) + for author_song in self.song.authors_songs: + self._add_author_to_list(author_song.author, author_song.author_type) # clear the results self.topics_list_view.clear() for topic in self.song.topics: @@ -496,6 +502,7 @@ class EditSongForm(QtGui.QDialog, Ui_EditSongDialog, RegistryProperties): """ item = int(self.authors_combo_box.currentIndex()) text = self.authors_combo_box.currentText().strip(' \r\n\t') + author_type = self.author_types_combo_box.itemData(self.author_types_combo_box.currentIndex()) # This if statement is for OS X, which doesn't seem to work well with # the QCompleter auto-completion class. See bug #812628. if text in self.authors: @@ -513,7 +520,7 @@ class EditSongForm(QtGui.QDialog, Ui_EditSongDialog, RegistryProperties): author = Author.populate(first_name=text.rsplit(' ', 1)[0], last_name=text.rsplit(' ', 1)[1], display_name=text) self.manager.save_object(author) - self._add_author_to_list(author) + self._add_author_to_list(author, author_type) self.load_authors() self.authors_combo_box.setCurrentIndex(0) else: @@ -525,7 +532,7 @@ class EditSongForm(QtGui.QDialog, Ui_EditSongDialog, RegistryProperties): critical_error_message_box( message=translate('SongsPlugin.EditSongForm', 'This author is already in the list.')) else: - self._add_author_to_list(author) + self._add_author_to_list(author, author_type) self.authors_combo_box.setCurrentIndex(0) else: QtGui.QMessageBox.warning( @@ -538,8 +545,7 @@ class EditSongForm(QtGui.QDialog, Ui_EditSongDialog, RegistryProperties): """ Run a set of actions when an author in the list is selected (mainly enable the delete button). """ - if self.authors_list_view.count() > 1: - self.author_remove_button.setEnabled(True) + self.author_remove_button.setEnabled(True) def on_author_remove_button_clicked(self): """ @@ -906,13 +912,13 @@ class EditSongForm(QtGui.QDialog, Ui_EditSongDialog, RegistryProperties): else: self.song.theme_name = None self._process_lyrics() - self.song.authors = [] + self.song.authors_songs = [] for row in range(self.authors_list_view.count()): item = self.authors_list_view.item(row) - author_id = (item.data(QtCore.Qt.UserRole)) - author = self.manager.get_object(Author, author_id) - if author is not None: - self.song.authors.append(author) + author_song = AuthorSong() + author_song.author_id = item.data(QtCore.Qt.UserRole)[0] + author_song.author_type = item.data(QtCore.Qt.UserRole)[1] + self.song.authors_songs.append(author_song) self.song.topics = [] for row in range(self.topics_list_view.count()): item = self.topics_list_view.item(row) diff --git a/openlp/plugins/songs/lib/__init__.py b/openlp/plugins/songs/lib/__init__.py index dc198d4b7..60e8788c8 100644 --- a/openlp/plugins/songs/lib/__init__.py +++ b/openlp/plugins/songs/lib/__init__.py @@ -389,13 +389,6 @@ def clean_song(manager, song): song.lyrics = str(song.lyrics, encoding='utf8') verses = SongXML().get_verses(song.lyrics) song.search_lyrics = ' '.join([clean_string(verse[1]) for verse in verses]) - # The song does not have any author, add one. - if not song.authors: - name = SongStrings.AuthorUnknown - author = manager.get_object_filtered(Author, Author.display_name == name) - if author is None: - author = Author.populate(display_name=name, last_name='', first_name='') - song.authors.append(author) if song.copyright: song.copyright = CONTROL_CHARS.sub('', song.copyright).strip() diff --git a/openlp/plugins/songs/lib/db.py b/openlp/plugins/songs/lib/db.py index c3965e2ed..5d360f4f1 100644 --- a/openlp/plugins/songs/lib/db.py +++ b/openlp/plugins/songs/lib/db.py @@ -39,12 +39,34 @@ from sqlalchemy.sql.expression import func from openlp.core.lib.db import BaseModel, init_db from openlp.core.utils import get_natural_key +from openlp.core.lib import translate class Author(BaseModel): """ Author model """ + #These types are defined by OpenLyrics: http://openlyrics.info/dataformat.html#authors + TYPE_WORDS = 'words' + TYPE_MUSIC = 'music' + TYPE_TRANSLATION = 'translation' + Types = { + TYPE_WORDS: translate('OpenLP.Ui', 'Words'), + TYPE_MUSIC: translate('OpenLP.Ui', 'Music'), + TYPE_TRANSLATION: translate('OpenLP.Ui', 'Translation') + } + def get_display_name(self, author_type=None): + if author_type: + return "%s: %s"%(self.Types[author_type], self.display_name) + return self.display_name + +class AuthorSong(BaseModel): + """ + Relationship between Authors and Songs (many to many). + Need to define this relationship table explicit to get access to the + Association Object (author_type). + http://docs.sqlalchemy.org/en/latest/orm/relationships.html#association-object + """ pass @@ -67,6 +89,7 @@ class Song(BaseModel): """ Song model """ + def __init__(self): self.sort_key = [] @@ -120,6 +143,7 @@ def init_schema(url): * author_id * song_id + * author_type **media_files Table** * id @@ -230,7 +254,8 @@ def init_schema(url): authors_songs_table = Table( 'authors_songs', metadata, Column('author_id', types.Integer(), ForeignKey('authors.id'), primary_key=True), - Column('song_id', types.Integer(), ForeignKey('songs.id'), primary_key=True) + Column('song_id', types.Integer(), ForeignKey('songs.id'), primary_key=True), + Column('author_type', types.String(), primary_key=True) ) # Definition of the "songs_topics" table @@ -241,10 +266,14 @@ def init_schema(url): ) mapper(Author, authors_table) + mapper(AuthorSong, authors_songs_table, properties={ + 'author': relation(Author) + }) mapper(Book, song_books_table) mapper(MediaFile, media_files_table) mapper(Song, songs_table, properties={ - 'authors': relation(Author, backref='songs', secondary=authors_songs_table, lazy=False), + 'authors_songs': relation(AuthorSong, cascade="all, delete-orphan"), + 'authors': relation(Author, secondary=authors_songs_table, viewonly=True), 'book': relation(Book, backref='songs'), 'media_files': relation(MediaFile, backref='songs', order_by=media_files_table.c.weight), 'topics': relation(Topic, backref='songs', secondary=songs_topics_table) diff --git a/openlp/plugins/songs/lib/mediaitem.py b/openlp/plugins/songs/lib/mediaitem.py index ad981135f..ca0c42483 100644 --- a/openlp/plugins/songs/lib/mediaitem.py +++ b/openlp/plugins/songs/lib/mediaitem.py @@ -464,23 +464,45 @@ class SongMediaItem(MediaManagerItem): def generate_footer(self, item, song): """ Generates the song footer based on a song and adds details to a service item. - author_list is only required for initial song generation. :param item: The service item to be amended :param song: The song to be used to generate the footer + :return List of all authors (only required for initial song generation) """ - author_list = [str(author.display_name) for author in song.authors] + authors_words = [] + authors_music = [] + authors_translation = [] + authors_none = [] + for author_song in song.authors_songs: + if author_song.author_type == Author.TYPE_WORDS: + authors_words.append(author_song.author.display_name) + elif author_song.author_type == Author.TYPE_MUSIC: + authors_music.append(author_song.author.display_name) + elif author_song.author_type == Author.TYPE_TRANSLATION: + authors_translation.append(author_song.author.display_name) + else: + authors_none.append(author_song.author.display_name) + authors_all = authors_words + authors_music + authors_translation + authors_none item.audit = [ - song.title, author_list, song.copyright, str(song.ccli_number) + song.title, authors_all, song.copyright, str(song.ccli_number) ] item.raw_footer = [] item.raw_footer.append(song.title) - item.raw_footer.append(create_separated_list(author_list)) + if authors_none: + item.raw_footer.append("%s: %s"%(translate('OpenLP.Ui', 'Written by'), create_separated_list(authors_words))) + if authors_words: + item.raw_footer.append("%s: %s"%(Author.Types[Author.TYPE_WORDS], create_separated_list(authors_words))) + if authors_music: + item.raw_footer.append("%s: %s"%(Author.Types[Author.TYPE_MUSIC], create_separated_list(authors_music))) + if authors_translation: + item.raw_footer.append("%s: %s"%(Author.Types[Author.TYPE_TRANSLATION], create_separated_list(authors_translation))) + if not authors_all: #No authors defined + item.raw_footer.append(SongStrings.AuthorUnknown) item.raw_footer.append(song.copyright) if Settings().value('core/ccli number'): item.raw_footer.append(translate('SongsPlugin.MediaItem', 'CCLI License: ') + Settings().value('core/ccli number')) - return author_list + return authors_all def service_load(self, item): """ diff --git a/openlp/plugins/songs/lib/upgrade.py b/openlp/plugins/songs/lib/upgrade.py index fdb0f17ef..15395fbb9 100644 --- a/openlp/plugins/songs/lib/upgrade.py +++ b/openlp/plugins/songs/lib/upgrade.py @@ -36,7 +36,7 @@ from sqlalchemy.sql.expression import func, false, null, text from openlp.core.lib.db import get_upgrade_op -__version__ = 3 +__version__ = 4 def upgrade_1(session, metadata): @@ -82,3 +82,12 @@ def upgrade_3(session, metadata): op.add_column('songs', Column('temporary', types.Boolean(create_constraint=False), server_default=false())) else: op.add_column('songs', Column('temporary', types.Boolean(), server_default=false())) + +def upgrade_4(session, metadata): + """ + Version 4 upgrade. + + This upgrade adds a column for author type to the authors table + """ + op = get_upgrade_op(session) + op.add_column('authors_songs', Column('author_type', types.String(), primary_key=True, server_default=null())) From 615442063e919730d40274a1886cc219c9d20183 Mon Sep 17 00:00:00 2001 From: Samuel Mehrbrodt Date: Sat, 29 Mar 2014 11:08:52 +0100 Subject: [PATCH 02/28] Fix display of Author type none --- openlp/plugins/songs/lib/mediaitem.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openlp/plugins/songs/lib/mediaitem.py b/openlp/plugins/songs/lib/mediaitem.py index ca0c42483..b71f27029 100644 --- a/openlp/plugins/songs/lib/mediaitem.py +++ b/openlp/plugins/songs/lib/mediaitem.py @@ -489,7 +489,7 @@ class SongMediaItem(MediaManagerItem): item.raw_footer = [] item.raw_footer.append(song.title) if authors_none: - item.raw_footer.append("%s: %s"%(translate('OpenLP.Ui', 'Written by'), create_separated_list(authors_words))) + item.raw_footer.append("%s: %s"%(translate('OpenLP.Ui', 'Written by'), create_separated_list(authors_none))) if authors_words: item.raw_footer.append("%s: %s"%(Author.Types[Author.TYPE_WORDS], create_separated_list(authors_words))) if authors_music: From 87051a094a2fc29a52ed0efad82b5708e3772816 Mon Sep 17 00:00:00 2001 From: Samuel Mehrbrodt Date: Sun, 30 Mar 2014 19:23:36 +0200 Subject: [PATCH 03/28] Put Author types in an own enum class --- openlp/plugins/songs/forms/editsongform.py | 8 ++++---- openlp/plugins/songs/lib/db.py | 24 +++++++++++++--------- openlp/plugins/songs/lib/mediaitem.py | 16 +++++++-------- 3 files changed, 26 insertions(+), 22 deletions(-) diff --git a/openlp/plugins/songs/forms/editsongform.py b/openlp/plugins/songs/forms/editsongform.py index ca17b98dc..2915ea4e8 100644 --- a/openlp/plugins/songs/forms/editsongform.py +++ b/openlp/plugins/songs/forms/editsongform.py @@ -42,7 +42,7 @@ from openlp.core.common import Registry, RegistryProperties, AppLocation, UiStri 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, AuthorSong, Topic, MediaFile +from openlp.plugins.songs.lib.db import Book, Song, Author, AuthorSong, AuthorType, Topic, MediaFile from openlp.plugins.songs.lib.ui import SongStrings from openlp.plugins.songs.lib.xml import SongXML from openlp.plugins.songs.forms.editsongdialog import Ui_EditSongDialog @@ -306,9 +306,9 @@ class EditSongForm(QtGui.QDialog, Ui_EditSongDialog, RegistryProperties): self.author_types_combo_box.clear() self.author_types_combo_box.addItem('') # Don't iterate over the dictionary to give them this specific order - self.author_types_combo_box.addItem(Author.Types[Author.TYPE_WORDS], Author.TYPE_WORDS) - self.author_types_combo_box.addItem(Author.Types[Author.TYPE_MUSIC], Author.TYPE_MUSIC) - self.author_types_combo_box.addItem(Author.Types[Author.TYPE_TRANSLATION], Author.TYPE_TRANSLATION) + self.author_types_combo_box.addItem(AuthorType.Types[AuthorType.Words], AuthorType.Words) + self.author_types_combo_box.addItem(AuthorType.Types[AuthorType.Music], AuthorType.Music) + self.author_types_combo_box.addItem(AuthorType.Types[AuthorType.Translation], AuthorType.Translation) def load_topics(self): """ diff --git a/openlp/plugins/songs/lib/db.py b/openlp/plugins/songs/lib/db.py index 5d360f4f1..0c1b3642c 100644 --- a/openlp/plugins/songs/lib/db.py +++ b/openlp/plugins/songs/lib/db.py @@ -46,18 +46,9 @@ class Author(BaseModel): """ Author model """ - #These types are defined by OpenLyrics: http://openlyrics.info/dataformat.html#authors - TYPE_WORDS = 'words' - TYPE_MUSIC = 'music' - TYPE_TRANSLATION = 'translation' - Types = { - TYPE_WORDS: translate('OpenLP.Ui', 'Words'), - TYPE_MUSIC: translate('OpenLP.Ui', 'Music'), - TYPE_TRANSLATION: translate('OpenLP.Ui', 'Translation') - } def get_display_name(self, author_type=None): if author_type: - return "%s: %s"%(self.Types[author_type], self.display_name) + return "%s: %s"%(AuthorType.Types[author_type], self.display_name) return self.display_name class AuthorSong(BaseModel): @@ -69,6 +60,19 @@ class AuthorSong(BaseModel): """ pass +class AuthorType(object): + """ + Enumeration for Author types. + They are defined by OpenLyrics: http://openlyrics.info/dataformat.html#authors + """ + Words = 'words' + Music = 'music' + Translation = 'translation' + Types = { + Words: translate('OpenLP.Ui', 'Words'), + Music: translate('OpenLP.Ui', 'Music'), + Translation: translate('OpenLP.Ui', 'Translation') + } class Book(BaseModel): """ diff --git a/openlp/plugins/songs/lib/mediaitem.py b/openlp/plugins/songs/lib/mediaitem.py index b71f27029..c40162474 100644 --- a/openlp/plugins/songs/lib/mediaitem.py +++ b/openlp/plugins/songs/lib/mediaitem.py @@ -44,7 +44,7 @@ from openlp.plugins.songs.forms.songmaintenanceform import SongMaintenanceForm 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, Song, Book, MediaFile +from openlp.plugins.songs.lib.db import Author, AuthorType, Song, Book, MediaFile from openlp.plugins.songs.lib.ui import SongStrings from openlp.plugins.songs.lib.xml import OpenLyrics, SongXML @@ -467,18 +467,18 @@ class SongMediaItem(MediaManagerItem): :param item: The service item to be amended :param song: The song to be used to generate the footer - :return List of all authors (only required for initial song generation) + :return: List of all authors (only required for initial song generation) """ authors_words = [] authors_music = [] authors_translation = [] authors_none = [] for author_song in song.authors_songs: - if author_song.author_type == Author.TYPE_WORDS: + if author_song.author_type == AuthorType.Words: authors_words.append(author_song.author.display_name) - elif author_song.author_type == Author.TYPE_MUSIC: + elif author_song.author_type == AuthorType.Music: authors_music.append(author_song.author.display_name) - elif author_song.author_type == Author.TYPE_TRANSLATION: + elif author_song.author_type == AuthorType.Translation: authors_translation.append(author_song.author.display_name) else: authors_none.append(author_song.author.display_name) @@ -491,11 +491,11 @@ class SongMediaItem(MediaManagerItem): if authors_none: item.raw_footer.append("%s: %s"%(translate('OpenLP.Ui', 'Written by'), create_separated_list(authors_none))) if authors_words: - item.raw_footer.append("%s: %s"%(Author.Types[Author.TYPE_WORDS], create_separated_list(authors_words))) + item.raw_footer.append("%s: %s"%(AuthorType.Types[AuthorType.Words], create_separated_list(authors_words))) if authors_music: - item.raw_footer.append("%s: %s"%(Author.Types[Author.TYPE_MUSIC], create_separated_list(authors_music))) + item.raw_footer.append("%s: %s"%(AuthorType.Types[AuthorType.Music], create_separated_list(authors_music))) if authors_translation: - item.raw_footer.append("%s: %s"%(Author.Types[Author.TYPE_TRANSLATION], create_separated_list(authors_translation))) + item.raw_footer.append("%s: %s"%(AuthorType.Types[AuthorType.Translation], create_separated_list(authors_translation))) if not authors_all: #No authors defined item.raw_footer.append(SongStrings.AuthorUnknown) item.raw_footer.append(song.copyright) From c7358e4a9f46f91f27e276f3949204228016b344 Mon Sep 17 00:00:00 2001 From: Samuel Mehrbrodt Date: Sun, 30 Mar 2014 19:38:26 +0200 Subject: [PATCH 04/28] The author_type column is part of the primary key and thus can't be NULL --- openlp/plugins/songs/forms/editsongform.py | 6 ------ openlp/plugins/songs/lib/db.py | 4 ++-- openlp/plugins/songs/lib/mediaitem.py | 4 ++-- openlp/plugins/songs/lib/upgrade.py | 5 +++-- 4 files changed, 7 insertions(+), 12 deletions(-) diff --git a/openlp/plugins/songs/forms/editsongform.py b/openlp/plugins/songs/forms/editsongform.py index 2915ea4e8..f3a695f03 100644 --- a/openlp/plugins/songs/forms/editsongform.py +++ b/openlp/plugins/songs/forms/editsongform.py @@ -214,12 +214,6 @@ class EditSongForm(QtGui.QDialog, Ui_EditSongDialog, RegistryProperties): critical_error_message_box( message=translate('SongsPlugin.EditSongForm', 'You need to type in at least one verse.')) return False - if self.authors_list_view.count() == 0: - self.song_tab_widget.setCurrentIndex(1) - self.authors_list_view.setFocus() - critical_error_message_box( - message=translate('SongsPlugin.EditSongForm', 'You need to have an author for this song.')) - return False if self.verse_order_edit.text(): result = self._validate_verse_list(self.verse_order_edit.text(), self.verse_list_widget.rowCount()) if not result: diff --git a/openlp/plugins/songs/lib/db.py b/openlp/plugins/songs/lib/db.py index 0c1b3642c..4e925e4b6 100644 --- a/openlp/plugins/songs/lib/db.py +++ b/openlp/plugins/songs/lib/db.py @@ -35,7 +35,7 @@ import re from sqlalchemy import Column, ForeignKey, Table, types from sqlalchemy.orm import mapper, relation, reconstructor -from sqlalchemy.sql.expression import func +from sqlalchemy.sql.expression import func, text from openlp.core.lib.db import BaseModel, init_db from openlp.core.utils import get_natural_key @@ -259,7 +259,7 @@ def init_schema(url): 'authors_songs', metadata, Column('author_id', types.Integer(), ForeignKey('authors.id'), primary_key=True), Column('song_id', types.Integer(), ForeignKey('songs.id'), primary_key=True), - Column('author_type', types.String(), primary_key=True) + Column('author_type', types.String(), primary_key=True, nullable=False, server_default=text('""')) ) # Definition of the "songs_topics" table diff --git a/openlp/plugins/songs/lib/mediaitem.py b/openlp/plugins/songs/lib/mediaitem.py index c40162474..ceb7848d6 100644 --- a/openlp/plugins/songs/lib/mediaitem.py +++ b/openlp/plugins/songs/lib/mediaitem.py @@ -234,8 +234,8 @@ class SongMediaItem(MediaManagerItem): if song.temporary: continue author_list = [author.display_name for author in song.authors] - song_title = str(song.title) - song_detail = '%s (%s)' % (song_title, create_separated_list(author_list)) + song_detail = '%s (%s)' % (song.title, create_separated_list(author_list)) if author_list\ + else '%s'%song.title song_name = QtGui.QListWidgetItem(song_detail) song_name.setData(QtCore.Qt.UserRole, song.id) self.list_view.addItem(song_name) diff --git a/openlp/plugins/songs/lib/upgrade.py b/openlp/plugins/songs/lib/upgrade.py index 87f960f3c..4ec4ca1c4 100644 --- a/openlp/plugins/songs/lib/upgrade.py +++ b/openlp/plugins/songs/lib/upgrade.py @@ -102,7 +102,8 @@ def upgrade_4(session, metadata): """ Version 4 upgrade. - This upgrade adds a column for author type to the authors table + This upgrade adds a column for author type to the authors_songs table """ op = get_upgrade_op(session) - op.add_column('authors_songs', Column('author_type', types.String(), primary_key=True, server_default=null())) + op.add_column('authors_songs', Column('author_type', types.String(), primary_key=True, + nullable=False, server_default=text('""'))) From c98970d0e4d050e53bde9e53b3bf1d358c8750d2 Mon Sep 17 00:00:00 2001 From: Samuel Mehrbrodt Date: Sun, 30 Mar 2014 20:01:03 +0200 Subject: [PATCH 05/28] Fix mediaitem tests --- openlp/plugins/songs/lib/db.py | 1 - .../openlp_plugins/songs/test_mediaitem.py | 42 ++++++++++++------- 2 files changed, 27 insertions(+), 16 deletions(-) diff --git a/openlp/plugins/songs/lib/db.py b/openlp/plugins/songs/lib/db.py index 4e925e4b6..204dac603 100644 --- a/openlp/plugins/songs/lib/db.py +++ b/openlp/plugins/songs/lib/db.py @@ -81,7 +81,6 @@ class Book(BaseModel): def __repr__(self): return '' % (str(self.id), self.name, self.publisher) - class MediaFile(BaseModel): """ MediaFile model diff --git a/tests/functional/openlp_plugins/songs/test_mediaitem.py b/tests/functional/openlp_plugins/songs/test_mediaitem.py index 2b5f02483..627663c62 100644 --- a/tests/functional/openlp_plugins/songs/test_mediaitem.py +++ b/tests/functional/openlp_plugins/songs/test_mediaitem.py @@ -10,6 +10,7 @@ from PyQt4 import QtCore, QtGui 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 tests.functional import patch, MagicMock from tests.helpers.testmixin import TestMixin @@ -45,10 +46,12 @@ class TestMediaItem(TestCase, TestMixin): # GIVEN: A Song and a Service Item mock_song = MagicMock() mock_song.title = 'My Song' + mock_song.authors_songs = [] mock_author = MagicMock() mock_author.display_name = 'my author' - mock_song.authors = [] - mock_song.authors.append(mock_author) + mock_author_song = MagicMock() + mock_author_song.author = mock_author + mock_song.authors_songs.append(mock_author_song) mock_song.copyright = 'My copyright' service_item = ServiceItem(None) @@ -56,7 +59,7 @@ class TestMediaItem(TestCase, TestMixin): author_list = self.media_item.generate_footer(service_item, mock_song) # THEN: I get the following Array returned - self.assertEqual(service_item.raw_footer, ['My Song', 'my author', 'My copyright'], + self.assertEqual(service_item.raw_footer, ['My Song', 'Written by: my author', 'My copyright'], 'The array should be returned correctly with a song, one author and copyright') self.assertEqual(author_list, ['my author'], 'The author list should be returned correctly with one author') @@ -68,13 +71,25 @@ class TestMediaItem(TestCase, TestMixin): # GIVEN: A Song and a Service Item mock_song = MagicMock() mock_song.title = 'My Song' + mock_song.authors_songs = [] mock_author = MagicMock() mock_author.display_name = 'my author' - mock_song.authors = [] - mock_song.authors.append(mock_author) + mock_author_song = MagicMock() + mock_author_song.author = mock_author + mock_author_song.author_type=AuthorType.Music + mock_song.authors_songs.append(mock_author_song) mock_author = MagicMock() mock_author.display_name = 'another author' - mock_song.authors.append(mock_author) + mock_author_song = MagicMock() + mock_author_song.author = mock_author + mock_author_song.author_type=AuthorType.Words + mock_song.authors_songs.append(mock_author_song) + mock_author = MagicMock() + mock_author.display_name = 'translator' + mock_author_song = MagicMock() + mock_author_song.author = mock_author + mock_author_song.author_type=AuthorType.Translation + mock_song.authors_songs.append(mock_author_song) mock_song.copyright = 'My copyright' service_item = ServiceItem(None) @@ -82,22 +97,19 @@ class TestMediaItem(TestCase, TestMixin): author_list = self.media_item.generate_footer(service_item, mock_song) # THEN: I get the following Array returned - self.assertEqual(service_item.raw_footer, ['My Song', 'my author and another author', 'My copyright'], + self.assertEqual(service_item.raw_footer, ['My Song', 'Words: another author', 'Music: my author', + 'Translation: translator', 'My copyright'], 'The array should be returned correctly with a song, two authors and copyright') - self.assertEqual(author_list, ['my author', 'another author'], + self.assertEqual(author_list, ['another author', 'my author', 'translator'], 'The author list should be returned correctly with two authors') def build_song_footer_base_ccli_test(self): """ - Test build songs footer with basic song and two authors + Test build songs footer with basic song and a CCLI number """ # GIVEN: A Song and a Service Item and a configured CCLI license mock_song = MagicMock() mock_song.title = 'My Song' - mock_author = MagicMock() - mock_author.display_name = 'my author' - mock_song.authors = [] - mock_song.authors.append(mock_author) mock_song.copyright = 'My copyright' service_item = ServiceItem(None) Settings().setValue('core/ccli number', '1234') @@ -106,7 +118,7 @@ class TestMediaItem(TestCase, TestMixin): self.media_item.generate_footer(service_item, mock_song) # THEN: I get the following Array returned - self.assertEqual(service_item.raw_footer, ['My Song', 'my author', 'My copyright', 'CCLI License: 1234'], + self.assertEqual(service_item.raw_footer, ['My Song', 'Author Unknown', 'My copyright', 'CCLI License: 1234'], 'The array should be returned correctly with a song, an author, copyright and ccli') # WHEN: I amend the CCLI value @@ -114,5 +126,5 @@ class TestMediaItem(TestCase, TestMixin): self.media_item.generate_footer(service_item, mock_song) # THEN: I would get an amended footer string - self.assertEqual(service_item.raw_footer, ['My Song', 'my author', 'My copyright', 'CCLI License: 4321'], + self.assertEqual(service_item.raw_footer, ['My Song', 'Author Unknown', 'My copyright', 'CCLI License: 4321'], 'The array should be returned correctly with a song, an author, copyright and amended ccli') From ad9717ea42e78bb6f66dc6c3bd9f43fe82f98d91 Mon Sep 17 00:00:00 2001 From: Samuel Mehrbrodt Date: Sun, 30 Mar 2014 23:42:46 +0200 Subject: [PATCH 06/28] Add support for author types to OpenLyrics import and export --- openlp/core/ui/wizard.py | 2 +- openlp/plugins/songs/lib/db.py | 3 ++- openlp/plugins/songs/lib/xml.py | 22 ++++++++++++++-------- 3 files changed, 17 insertions(+), 10 deletions(-) diff --git a/openlp/core/ui/wizard.py b/openlp/core/ui/wizard.py index 05951d14a..5815457b5 100644 --- a/openlp/core/ui/wizard.py +++ b/openlp/core/ui/wizard.py @@ -197,7 +197,7 @@ class OpenLPWizard(QtGui.QWizard, RegistryProperties): """ Run the wizard. """ - self.setDefaults() + self.set_defaults() return QtGui.QWizard.exec_(self) def reject(self): diff --git a/openlp/plugins/songs/lib/db.py b/openlp/plugins/songs/lib/db.py index 204dac603..f597fc2d2 100644 --- a/openlp/plugins/songs/lib/db.py +++ b/openlp/plugins/songs/lib/db.py @@ -275,8 +275,9 @@ def init_schema(url): mapper(Book, song_books_table) mapper(MediaFile, media_files_table) mapper(Song, songs_table, properties={ + # Use the authors_songs relation when you need access to the 'author_type' attribute. 'authors_songs': relation(AuthorSong, cascade="all, delete-orphan"), - 'authors': relation(Author, secondary=authors_songs_table, viewonly=True), + 'authors': relation(Author, secondary=authors_songs_table), 'book': relation(Book, backref='songs'), 'media_files': relation(MediaFile, backref='songs', order_by=media_files_table.c.weight), 'topics': relation(Topic, backref='songs', secondary=songs_topics_table) diff --git a/openlp/plugins/songs/lib/xml.py b/openlp/plugins/songs/lib/xml.py index 667afebdd..7e41008bd 100644 --- a/openlp/plugins/songs/lib/xml.py +++ b/openlp/plugins/songs/lib/xml.py @@ -71,7 +71,7 @@ from lxml import etree, objectify from openlp.core.common import translate from openlp.core.lib import FormattingTags from openlp.plugins.songs.lib import VerseType, clean_song -from openlp.plugins.songs.lib.db import Author, Book, Song, Topic +from openlp.plugins.songs.lib.db import Author, AuthorSong, Book, Song, Topic from openlp.core.utils import get_application_version log = logging.getLogger(__name__) @@ -166,7 +166,7 @@ class OpenLyrics(object): supported by the :class:`OpenLyrics` class: ```` - OpenLP does not support the attribute *type* and *lang*. + OpenLP does not support the attribute *lang*. ```` This property is not supported. @@ -269,10 +269,12 @@ class OpenLyrics(object): 'verseOrder', properties, song.verse_order.lower()) if song.ccli_number: self._add_text_to_element('ccliNo', properties, song.ccli_number) - if song.authors: + if song.authors_songs: authors = etree.SubElement(properties, 'authors') - for author in song.authors: - self._add_text_to_element('author', authors, author.display_name) + for author_song in song.authors_songs: + element = self._add_text_to_element('author', authors, author_song.author.display_name) + if author_song.author_type: + 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 @@ -501,16 +503,20 @@ class OpenLyrics(object): if hasattr(properties, 'authors'): for author in properties.authors.author: display_name = self._text(author) + author_type = author.get('type', '') if display_name: - authors.append(display_name) - for display_name in authors: + authors.append((display_name, author_type)) + for (display_name, author_type) in authors: author = self.manager.get_object_filtered(Author, Author.display_name == display_name) if author is None: # We need to create a new author, as the author does not exist. author = Author.populate(display_name=display_name, last_name=display_name.split(' ')[-1], first_name=' '.join(display_name.split(' ')[:-1])) - song.authors.append(author) + author_song = AuthorSong() + author_song.author = author + author_song.author_type = author_type + song.authors_songs.append(author_song) def _process_cclinumber(self, properties, song): """ From 88c641715caee5b3683ed71c4cf6eeeb4bee40f9 Mon Sep 17 00:00:00 2001 From: Samuel Mehrbrodt Date: Sun, 30 Mar 2014 23:45:52 +0200 Subject: [PATCH 07/28] Translate this string --- openlp/plugins/songs/lib/ui.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openlp/plugins/songs/lib/ui.py b/openlp/plugins/songs/lib/ui.py index 14f4777c9..4f60bec1f 100644 --- a/openlp/plugins/songs/lib/ui.py +++ b/openlp/plugins/songs/lib/ui.py @@ -40,7 +40,7 @@ class SongStrings(object): # These strings should need a good reason to be retranslated elsewhere. Author = translate('OpenLP.Ui', 'Author', 'Singular') Authors = translate('OpenLP.Ui', 'Authors', 'Plural') - AuthorUnknown = 'Author Unknown' # Used to populate the database. + AuthorUnknown = translate('OpenLP.Ui', 'Author Unknown') #Shown when a song has no author CopyrightSymbol = translate('OpenLP.Ui', '\xa9', 'Copyright symbol.') SongBook = translate('OpenLP.Ui', 'Song Book', 'Singular') SongBooks = translate('OpenLP.Ui', 'Song Books', 'Plural') From ad982e9fa31b0da6f2f54b4b01c3c332bb463c1f Mon Sep 17 00:00:00 2001 From: Samuel Mehrbrodt Date: Sun, 30 Mar 2014 23:49:55 +0200 Subject: [PATCH 08/28] Add try/catch block to upgrade function until better solution is found --- openlp/plugins/songs/lib/upgrade.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/openlp/plugins/songs/lib/upgrade.py b/openlp/plugins/songs/lib/upgrade.py index 4ec4ca1c4..d0123feaa 100644 --- a/openlp/plugins/songs/lib/upgrade.py +++ b/openlp/plugins/songs/lib/upgrade.py @@ -104,6 +104,9 @@ def upgrade_4(session, metadata): This upgrade adds a column for author type to the authors_songs table """ - op = get_upgrade_op(session) - op.add_column('authors_songs', Column('author_type', types.String(), primary_key=True, - nullable=False, server_default=text('""'))) + try: + op = get_upgrade_op(session) + op.add_column('authors_songs', Column('author_type', types.String(), primary_key=True, + nullable=False, server_default=text('""'))) + except OperationalError: + log.info('Upgrade 4 has already been run') From 830e3d6624c717cf140e013206c65c096c874ccf Mon Sep 17 00:00:00 2001 From: Samuel Mehrbrodt Date: Tue, 1 Apr 2014 23:07:49 +0200 Subject: [PATCH 09/28] PEP8 --- openlp/plugins/songs/lib/db.py | 6 +++++- openlp/plugins/songs/lib/mediaitem.py | 17 ++++++++++------- openlp/plugins/songs/lib/ui.py | 2 +- openlp/plugins/songs/lib/upgrade.py | 1 + 4 files changed, 17 insertions(+), 9 deletions(-) diff --git a/openlp/plugins/songs/lib/db.py b/openlp/plugins/songs/lib/db.py index f597fc2d2..59592a7e0 100644 --- a/openlp/plugins/songs/lib/db.py +++ b/openlp/plugins/songs/lib/db.py @@ -48,9 +48,10 @@ class Author(BaseModel): """ def get_display_name(self, author_type=None): if author_type: - return "%s: %s"%(AuthorType.Types[author_type], self.display_name) + return "%s: %s" % (AuthorType.Types[author_type], self.display_name) return self.display_name + class AuthorSong(BaseModel): """ Relationship between Authors and Songs (many to many). @@ -60,6 +61,7 @@ class AuthorSong(BaseModel): """ pass + class AuthorType(object): """ Enumeration for Author types. @@ -74,6 +76,7 @@ class AuthorType(object): Translation: translate('OpenLP.Ui', 'Translation') } + class Book(BaseModel): """ Book model @@ -81,6 +84,7 @@ class Book(BaseModel): def __repr__(self): return '' % (str(self.id), self.name, self.publisher) + class MediaFile(BaseModel): """ MediaFile model diff --git a/openlp/plugins/songs/lib/mediaitem.py b/openlp/plugins/songs/lib/mediaitem.py index ceb7848d6..71c5fea9c 100644 --- a/openlp/plugins/songs/lib/mediaitem.py +++ b/openlp/plugins/songs/lib/mediaitem.py @@ -234,8 +234,7 @@ class SongMediaItem(MediaManagerItem): if song.temporary: continue author_list = [author.display_name for author in song.authors] - song_detail = '%s (%s)' % (song.title, create_separated_list(author_list)) if author_list\ - else '%s'%song.title + song_detail = '%s (%s)' % (song.title, create_separated_list(author_list)) if author_list else song.title song_name = QtGui.QListWidgetItem(song_detail) song_name.setData(QtCore.Qt.UserRole, song.id) self.list_view.addItem(song_name) @@ -489,14 +488,18 @@ class SongMediaItem(MediaManagerItem): item.raw_footer = [] item.raw_footer.append(song.title) if authors_none: - item.raw_footer.append("%s: %s"%(translate('OpenLP.Ui', 'Written by'), create_separated_list(authors_none))) + item.raw_footer.append("%s: %s" % (translate('OpenLP.Ui', 'Written by'), + create_separated_list(authors_none))) if authors_words: - item.raw_footer.append("%s: %s"%(AuthorType.Types[AuthorType.Words], create_separated_list(authors_words))) + item.raw_footer.append("%s: %s" % (AuthorType.Types[AuthorType.Words], + create_separated_list(authors_words))) if authors_music: - item.raw_footer.append("%s: %s"%(AuthorType.Types[AuthorType.Music], create_separated_list(authors_music))) + item.raw_footer.append("%s: %s" % (AuthorType.Types[AuthorType.Music], + create_separated_list(authors_music))) if authors_translation: - item.raw_footer.append("%s: %s"%(AuthorType.Types[AuthorType.Translation], create_separated_list(authors_translation))) - if not authors_all: #No authors defined + item.raw_footer.append("%s: %s" % (AuthorType.Types[AuthorType.Translation], + create_separated_list(authors_translation))) + if not authors_all: # No authors defined item.raw_footer.append(SongStrings.AuthorUnknown) item.raw_footer.append(song.copyright) if Settings().value('core/ccli number'): diff --git a/openlp/plugins/songs/lib/ui.py b/openlp/plugins/songs/lib/ui.py index 4f60bec1f..193b70608 100644 --- a/openlp/plugins/songs/lib/ui.py +++ b/openlp/plugins/songs/lib/ui.py @@ -40,7 +40,7 @@ class SongStrings(object): # These strings should need a good reason to be retranslated elsewhere. Author = translate('OpenLP.Ui', 'Author', 'Singular') Authors = translate('OpenLP.Ui', 'Authors', 'Plural') - AuthorUnknown = translate('OpenLP.Ui', 'Author Unknown') #Shown when a song has no author + AuthorUnknown = translate('OpenLP.Ui', 'Author Unknown') # Shown when a song has no author CopyrightSymbol = translate('OpenLP.Ui', '\xa9', 'Copyright symbol.') SongBook = translate('OpenLP.Ui', 'Song Book', 'Singular') SongBooks = translate('OpenLP.Ui', 'Song Books', 'Plural') diff --git a/openlp/plugins/songs/lib/upgrade.py b/openlp/plugins/songs/lib/upgrade.py index d0123feaa..96cd65051 100644 --- a/openlp/plugins/songs/lib/upgrade.py +++ b/openlp/plugins/songs/lib/upgrade.py @@ -98,6 +98,7 @@ def upgrade_3(session, metadata): except OperationalError: log.info('Upgrade 3 has already been run') + def upgrade_4(session, metadata): """ Version 4 upgrade. From 691247ffda9ac8a91ad62af6742d275087d39c33 Mon Sep 17 00:00:00 2001 From: Samuel Mehrbrodt Date: Wed, 2 Apr 2014 08:47:27 +0200 Subject: [PATCH 10/28] Don't allow a song to be saved without an author --- openlp/plugins/songs/forms/editsongform.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/openlp/plugins/songs/forms/editsongform.py b/openlp/plugins/songs/forms/editsongform.py index f3a695f03..ebf967755 100644 --- a/openlp/plugins/songs/forms/editsongform.py +++ b/openlp/plugins/songs/forms/editsongform.py @@ -214,6 +214,12 @@ class EditSongForm(QtGui.QDialog, Ui_EditSongDialog, RegistryProperties): critical_error_message_box( message=translate('SongsPlugin.EditSongForm', 'You need to type in at least one verse.')) return False + if self.authors_list_view.count() == 0: + self.song_tab_widget.setCurrentIndex(1) + self.authors_list_view.setFocus() + critical_error_message_box( + message=translate('SongsPlugin.EditSongForm', 'You need to have an author for this song.')) + return False if self.verse_order_edit.text(): result = self._validate_verse_list(self.verse_order_edit.text(), self.verse_list_widget.rowCount()) if not result: From 31b220c9726da648ecb3202b8d0c16c25fc9de6e Mon Sep 17 00:00:00 2001 From: Samuel Mehrbrodt Date: Wed, 2 Apr 2014 08:49:03 +0200 Subject: [PATCH 11/28] Indentation --- openlp/plugins/songs/forms/editsongform.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/openlp/plugins/songs/forms/editsongform.py b/openlp/plugins/songs/forms/editsongform.py index ebf967755..6417b4a73 100644 --- a/openlp/plugins/songs/forms/editsongform.py +++ b/openlp/plugins/songs/forms/editsongform.py @@ -217,8 +217,8 @@ class EditSongForm(QtGui.QDialog, Ui_EditSongDialog, RegistryProperties): if self.authors_list_view.count() == 0: self.song_tab_widget.setCurrentIndex(1) self.authors_list_view.setFocus() - critical_error_message_box( - message=translate('SongsPlugin.EditSongForm', 'You need to have an author for this song.')) + critical_error_message_box(message=translate('SongsPlugin.EditSongForm', + 'You need to have an author for this song.')) return False if self.verse_order_edit.text(): result = self._validate_verse_list(self.verse_order_edit.text(), self.verse_list_widget.rowCount()) From dffee7fbc159e8ce776aec02ec1370af8f9bb45a Mon Sep 17 00:00:00 2001 From: Samuel Mehrbrodt Date: Wed, 2 Apr 2014 09:03:53 +0200 Subject: [PATCH 12/28] Restore setting a default author --- openlp/plugins/songs/lib/__init__.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/openlp/plugins/songs/lib/__init__.py b/openlp/plugins/songs/lib/__init__.py index 60e8788c8..aa9fbc4c9 100644 --- a/openlp/plugins/songs/lib/__init__.py +++ b/openlp/plugins/songs/lib/__init__.py @@ -389,6 +389,13 @@ def clean_song(manager, song): song.lyrics = str(song.lyrics, encoding='utf8') verses = SongXML().get_verses(song.lyrics) song.search_lyrics = ' '.join([clean_string(verse[1]) for verse in verses]) + # The song does not have any author, add one. + if not song.authors and not song.authors_songs: # Need to check both relations + name = SongStrings.AuthorUnknown + author = manager.get_object_filtered(Author, Author.display_name == name) + if author is None: + author = Author.populate(display_name=name, last_name='', first_name='') + song.authors.append(author) if song.copyright: song.copyright = CONTROL_CHARS.sub('', song.copyright).strip() From adedb6eaebb1b20f3365ff3628818151fd037557 Mon Sep 17 00:00:00 2001 From: Samuel Mehrbrodt Date: Wed, 2 Apr 2014 09:04:12 +0200 Subject: [PATCH 13/28] Fix setDefaults -> set_defaults --- openlp/core/ui/firsttimeform.py | 4 ++-- openlp/core/ui/themeform.py | 4 ++-- openlp/plugins/bibles/forms/bibleimportform.py | 2 +- openlp/plugins/bibles/forms/bibleupgradeform.py | 2 +- openlp/plugins/songs/forms/duplicatesongremovalform.py | 2 +- openlp/plugins/songs/forms/songimportform.py | 2 +- 6 files changed, 8 insertions(+), 8 deletions(-) diff --git a/openlp/core/ui/firsttimeform.py b/openlp/core/ui/firsttimeform.py index aa89da6c0..fc6a61db6 100644 --- a/openlp/core/ui/firsttimeform.py +++ b/openlp/core/ui/firsttimeform.py @@ -110,10 +110,10 @@ class FirstTimeForm(QtGui.QWizard, Ui_FirstTimeWizard, RegistryProperties): """ Run the wizard. """ - self.setDefaults() + self.set_defaults() return QtGui.QWizard.exec_(self) - def setDefaults(self): + def set_defaults(self): """ Set up display at start of theme edit. """ diff --git a/openlp/core/ui/themeform.py b/openlp/core/ui/themeform.py index fbfc1035c..c8182232b 100644 --- a/openlp/core/ui/themeform.py +++ b/openlp/core/ui/themeform.py @@ -90,7 +90,7 @@ class ThemeForm(QtGui.QWizard, Ui_ThemeWizard, RegistryProperties): self.footer_font_combo_box.activated.connect(self.update_theme) self.footer_size_spin_box.valueChanged.connect(self.update_theme) - def setDefaults(self): + def set_defaults(self): """ Set up display at start of theme edit. """ @@ -261,7 +261,7 @@ class ThemeForm(QtGui.QWizard, Ui_ThemeWizard, RegistryProperties): log.debug('Editing theme %s' % self.theme.theme_name) self.temp_background_filename = '' self.update_theme_allowed = False - self.setDefaults() + self.set_defaults() self.update_theme_allowed = True self.theme_name_label.setVisible(not edit) self.theme_name_edit.setVisible(not edit) diff --git a/openlp/plugins/bibles/forms/bibleimportform.py b/openlp/plugins/bibles/forms/bibleimportform.py index ee5bee2d0..79b0bc699 100644 --- a/openlp/plugins/bibles/forms/bibleimportform.py +++ b/openlp/plugins/bibles/forms/bibleimportform.py @@ -465,7 +465,7 @@ class BibleImportForm(OpenLPWizard): self.license_details_page.registerField('license_copyright', self.copyright_edit) self.license_details_page.registerField('license_permissions', self.permissions_edit) - def setDefaults(self): + def set_defaults(self): """ Set default values for the wizard pages. """ diff --git a/openlp/plugins/bibles/forms/bibleupgradeform.py b/openlp/plugins/bibles/forms/bibleupgradeform.py index 9925b1ebc..0c004da8a 100644 --- a/openlp/plugins/bibles/forms/bibleupgradeform.py +++ b/openlp/plugins/bibles/forms/bibleupgradeform.py @@ -307,7 +307,7 @@ class BibleUpgradeForm(OpenLPWizard): if self.currentPage() == self.progress_page: return True - def setDefaults(self): + def set_defaults(self): """ Set default values for the wizard pages. """ diff --git a/openlp/plugins/songs/forms/duplicatesongremovalform.py b/openlp/plugins/songs/forms/duplicatesongremovalform.py index 22299cde5..8ca673448 100644 --- a/openlp/plugins/songs/forms/duplicatesongremovalform.py +++ b/openlp/plugins/songs/forms/duplicatesongremovalform.py @@ -244,7 +244,7 @@ class DuplicateSongRemovalForm(OpenLPWizard, RegistryProperties): self.break_search = True self.plugin.media_item.on_search_text_button_clicked() - def setDefaults(self): + def set_defaults(self): """ Set default form values for the song import wizard. """ diff --git a/openlp/plugins/songs/forms/songimportform.py b/openlp/plugins/songs/forms/songimportform.py index 27f0d9343..21569a034 100644 --- a/openlp/plugins/songs/forms/songimportform.py +++ b/openlp/plugins/songs/forms/songimportform.py @@ -304,7 +304,7 @@ class SongImportForm(OpenLPWizard, RegistryProperties): """ self.source_page.emit(QtCore.SIGNAL('completeChanged()')) - def setDefaults(self): + def set_defaults(self): """ Set default form values for the song import wizard. """ From d62cd37db4fe7347516967e59b328ff3cb289f55 Mon Sep 17 00:00:00 2001 From: Samuel Mehrbrodt Date: Wed, 2 Apr 2014 09:07:46 +0200 Subject: [PATCH 14/28] Restore comment --- openlp/plugins/songs/lib/ui.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openlp/plugins/songs/lib/ui.py b/openlp/plugins/songs/lib/ui.py index 193b70608..151b11b4b 100644 --- a/openlp/plugins/songs/lib/ui.py +++ b/openlp/plugins/songs/lib/ui.py @@ -40,7 +40,7 @@ class SongStrings(object): # These strings should need a good reason to be retranslated elsewhere. Author = translate('OpenLP.Ui', 'Author', 'Singular') Authors = translate('OpenLP.Ui', 'Authors', 'Plural') - AuthorUnknown = translate('OpenLP.Ui', 'Author Unknown') # Shown when a song has no author + 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') From cc635e9b9678b95ecfc3c3cdf42dd9e0150da2d5 Mon Sep 17 00:00:00 2001 From: Samuel Mehrbrodt Date: Tue, 8 Apr 2014 11:16:14 +0200 Subject: [PATCH 15/28] SQLite doesn't support changing a primary key --- openlp/plugins/songs/forms/editsongform.py | 2 +- openlp/plugins/songs/lib/upgrade.py | 13 ++++++++++--- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/openlp/plugins/songs/forms/editsongform.py b/openlp/plugins/songs/forms/editsongform.py index 6417b4a73..ba4298922 100644 --- a/openlp/plugins/songs/forms/editsongform.py +++ b/openlp/plugins/songs/forms/editsongform.py @@ -528,7 +528,7 @@ class EditSongForm(QtGui.QDialog, Ui_EditSongDialog, RegistryProperties): elif item > 0: item_id = (self.authors_combo_box.itemData(item)) author = self.manager.get_object(Author, item_id) - if self.authors_list_view.findItems(str(author.display_name), QtCore.Qt.MatchExactly): + if self.authors_list_view.findItems(author.get_display_name(author_type), QtCore.Qt.MatchExactly): critical_error_message_box( message=translate('SongsPlugin.EditSongForm', 'This author is already in the list.')) else: diff --git a/openlp/plugins/songs/lib/upgrade.py b/openlp/plugins/songs/lib/upgrade.py index 96cd65051..59355bbec 100644 --- a/openlp/plugins/songs/lib/upgrade.py +++ b/openlp/plugins/songs/lib/upgrade.py @@ -32,7 +32,7 @@ backend for the Songs plugin """ import logging -from sqlalchemy import Column, types +from sqlalchemy import Column, ForeignKey, types from sqlalchemy.exc import OperationalError from sqlalchemy.sql.expression import func, false, null, text @@ -106,8 +106,15 @@ def upgrade_4(session, metadata): This upgrade adds a column for author type to the authors_songs table """ try: + # Since SQLite doesn't support changing the primary key of a table, we need to recreate the table + # and copy the old values op = get_upgrade_op(session) - op.add_column('authors_songs', Column('author_type', types.String(), primary_key=True, - nullable=False, server_default=text('""'))) + op.create_table('authors_songs_tmp', + Column('author_id', types.Integer(), ForeignKey('authors.id'), primary_key=True), + Column('song_id', types.Integer(), ForeignKey('songs.id'), primary_key=True), + Column('author_type', types.String(), primary_key=True, nullable=False, server_default=text('""'))) + op.execute('INSERT INTO authors_songs_tmp SELECT author_id, song_id, "" FROM authors_songs') + op.drop_table('authors_songs') + op.rename_table('authors_songs_tmp', 'authors_songs') except OperationalError: log.info('Upgrade 4 has already been run') From 7f8e76b8ddcb0d814698e2119f39845d9fd9e250 Mon Sep 17 00:00:00 2001 From: Samuel Mehrbrodt Date: Tue, 8 Apr 2014 11:21:56 +0200 Subject: [PATCH 16/28] Restore previous delete button behavior --- openlp/plugins/songs/forms/editsongform.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/openlp/plugins/songs/forms/editsongform.py b/openlp/plugins/songs/forms/editsongform.py index ba4298922..c5ddb2a62 100644 --- a/openlp/plugins/songs/forms/editsongform.py +++ b/openlp/plugins/songs/forms/editsongform.py @@ -545,7 +545,8 @@ class EditSongForm(QtGui.QDialog, Ui_EditSongDialog, RegistryProperties): """ Run a set of actions when an author in the list is selected (mainly enable the delete button). """ - self.author_remove_button.setEnabled(True) + if self.authors_list_view.count() > 1: + self.author_remove_button.setEnabled(True) def on_author_remove_button_clicked(self): """ From fb85b9858a2094198dcb31cfb54d9df1c7232059 Mon Sep 17 00:00:00 2001 From: Samuel Mehrbrodt Date: Tue, 8 Apr 2014 11:29:47 +0200 Subject: [PATCH 17/28] PEP8 --- openlp/plugins/songs/forms/editsongform.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/openlp/plugins/songs/forms/editsongform.py b/openlp/plugins/songs/forms/editsongform.py index c5ddb2a62..d27f43343 100644 --- a/openlp/plugins/songs/forms/editsongform.py +++ b/openlp/plugins/songs/forms/editsongform.py @@ -302,7 +302,7 @@ class EditSongForm(QtGui.QDialog, Ui_EditSongDialog, RegistryProperties): self.authors.append(author.display_name) set_case_insensitive_completer(self.authors, self.authors_combo_box) - #Types + # Types self.author_types_combo_box.clear() self.author_types_combo_box.addItem('') # Don't iterate over the dictionary to give them this specific order @@ -689,7 +689,7 @@ class EditSongForm(QtGui.QDialog, Ui_EditSongDialog, RegistryProperties): verse_index = VerseType.from_loose_input(verse_name) verse_tag = VerseType.tags[verse_index] # Later we need to handle v1a as well. - #regex = re.compile(r'(\d+\w.)') + # regex = re.compile(r'(\d+\w.)') regex = re.compile(r'\D*(\d+)\D*') match = regex.match(verse_num) if match: From 577c7321ffec12c4895d30415bfbcda1f22f8952 Mon Sep 17 00:00:00 2001 From: Samuel Mehrbrodt Date: Tue, 8 Apr 2014 20:52:05 +0200 Subject: [PATCH 18/28] Add new type for 'Words and Music' --- openlp/plugins/songs/forms/editsongform.py | 1 + openlp/plugins/songs/lib/db.py | 4 ++++ openlp/plugins/songs/lib/xml.py | 10 ++++++++-- 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/openlp/plugins/songs/forms/editsongform.py b/openlp/plugins/songs/forms/editsongform.py index d27f43343..4d6e1addb 100644 --- a/openlp/plugins/songs/forms/editsongform.py +++ b/openlp/plugins/songs/forms/editsongform.py @@ -308,6 +308,7 @@ class EditSongForm(QtGui.QDialog, Ui_EditSongDialog, RegistryProperties): # Don't iterate over the dictionary to give them this specific order self.author_types_combo_box.addItem(AuthorType.Types[AuthorType.Words], AuthorType.Words) self.author_types_combo_box.addItem(AuthorType.Types[AuthorType.Music], AuthorType.Music) + self.author_types_combo_box.addItem(AuthorType.Types[AuthorType.WordsAndMusic], AuthorType.WordsAndMusic) self.author_types_combo_box.addItem(AuthorType.Types[AuthorType.Translation], AuthorType.Translation) def load_topics(self): diff --git a/openlp/plugins/songs/lib/db.py b/openlp/plugins/songs/lib/db.py index 59592a7e0..fa53bccc8 100644 --- a/openlp/plugins/songs/lib/db.py +++ b/openlp/plugins/songs/lib/db.py @@ -66,13 +66,17 @@ class AuthorType(object): """ Enumeration for Author types. They are defined by OpenLyrics: http://openlyrics.info/dataformat.html#authors + + The 'words+music' type is not an official type, but is provided for convenience. """ Words = 'words' Music = 'music' + WordsAndMusic = 'words+music' Translation = 'translation' Types = { Words: translate('OpenLP.Ui', 'Words'), Music: translate('OpenLP.Ui', 'Music'), + WordsAndMusic: translate('OpenLP.Ui', 'Words and Music'), Translation: translate('OpenLP.Ui', 'Translation') } diff --git a/openlp/plugins/songs/lib/xml.py b/openlp/plugins/songs/lib/xml.py index 7e41008bd..64c9aaf05 100644 --- a/openlp/plugins/songs/lib/xml.py +++ b/openlp/plugins/songs/lib/xml.py @@ -71,7 +71,7 @@ from lxml import etree, objectify from openlp.core.common import translate from openlp.core.lib import FormattingTags from openlp.plugins.songs.lib import VerseType, clean_song -from openlp.plugins.songs.lib.db import Author, AuthorSong, Book, Song, Topic +from openlp.plugins.songs.lib.db import Author, AuthorSong, AuthorType, Book, Song, Topic from openlp.core.utils import get_application_version log = logging.getLogger(__name__) @@ -274,7 +274,13 @@ class OpenLyrics(object): for author_song in song.authors_songs: element = self._add_text_to_element('author', authors, author_song.author.display_name) if author_song.author_type: - element.set('type', author_song.author_type) + # Handle the special case 'words+music': Need to create two separate authors for that + if author_song.author_type == AuthorType.WordsAndMusic: + element.set('type', AuthorType.Words) + element = self._add_text_to_element('author', authors, author_song.author.display_name) + 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 From efad4a55fae57ef56c7e8e9ce6b3e624218582ea Mon Sep 17 00:00:00 2001 From: Samuel Mehrbrodt Date: Tue, 8 Apr 2014 20:57:01 +0200 Subject: [PATCH 19/28] Display 'Words and Music' type in the footer --- openlp/plugins/songs/lib/mediaitem.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/openlp/plugins/songs/lib/mediaitem.py b/openlp/plugins/songs/lib/mediaitem.py index 71c5fea9c..af7157a63 100644 --- a/openlp/plugins/songs/lib/mediaitem.py +++ b/openlp/plugins/songs/lib/mediaitem.py @@ -470,6 +470,7 @@ class SongMediaItem(MediaManagerItem): """ authors_words = [] authors_music = [] + authors_words_music = [] authors_translation = [] authors_none = [] for author_song in song.authors_songs: @@ -477,11 +478,13 @@ class SongMediaItem(MediaManagerItem): authors_words.append(author_song.author.display_name) elif author_song.author_type == AuthorType.Music: authors_music.append(author_song.author.display_name) + elif author_song.author_type == AuthorType.WordsAndMusic: + authors_words_music.append(author_song.author.display_name) elif author_song.author_type == AuthorType.Translation: authors_translation.append(author_song.author.display_name) else: authors_none.append(author_song.author.display_name) - authors_all = authors_words + authors_music + authors_translation + authors_none + authors_all = authors_words_music + authors_words + authors_music + authors_translation + authors_none item.audit = [ song.title, authors_all, song.copyright, str(song.ccli_number) ] @@ -490,6 +493,9 @@ class SongMediaItem(MediaManagerItem): if authors_none: item.raw_footer.append("%s: %s" % (translate('OpenLP.Ui', 'Written by'), create_separated_list(authors_none))) + if authors_words_music: + item.raw_footer.append("%s: %s" % (AuthorType.Types[AuthorType.WordsAndMusic], + create_separated_list(authors_words_music))) if authors_words: item.raw_footer.append("%s: %s" % (AuthorType.Types[AuthorType.Words], create_separated_list(authors_words))) @@ -499,8 +505,6 @@ class SongMediaItem(MediaManagerItem): if authors_translation: item.raw_footer.append("%s: %s" % (AuthorType.Types[AuthorType.Translation], create_separated_list(authors_translation))) - if not authors_all: # No authors defined - item.raw_footer.append(SongStrings.AuthorUnknown) item.raw_footer.append(song.copyright) if Settings().value('core/ccli number'): item.raw_footer.append(translate('SongsPlugin.MediaItem', From abf1671754bf008874bcaea1850a6b17eaf9bcee Mon Sep 17 00:00:00 2001 From: Samuel Mehrbrodt Date: Tue, 8 Apr 2014 20:58:28 +0200 Subject: [PATCH 20/28] Ignore Komodo project and project directory --- .bzrignore | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.bzrignore b/.bzrignore index f149d97a7..716add8bb 100644 --- a/.bzrignore +++ b/.bzrignore @@ -5,6 +5,8 @@ *.ropeproject *.e4* .eric4project +.komodotools +*.komodoproject list openlp.org 2.0.e4* documentation/build/html From e0c638176278a8c2990325b6ea5eee0f11af4c04 Mon Sep 17 00:00:00 2001 From: Samuel Mehrbrodt Date: Tue, 8 Apr 2014 21:02:13 +0200 Subject: [PATCH 21/28] Fix test --- tests/functional/openlp_plugins/songs/test_mediaitem.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/functional/openlp_plugins/songs/test_mediaitem.py b/tests/functional/openlp_plugins/songs/test_mediaitem.py index 627663c62..1b372aa20 100644 --- a/tests/functional/openlp_plugins/songs/test_mediaitem.py +++ b/tests/functional/openlp_plugins/songs/test_mediaitem.py @@ -118,7 +118,7 @@ class TestMediaItem(TestCase, TestMixin): self.media_item.generate_footer(service_item, mock_song) # THEN: I get the following Array returned - self.assertEqual(service_item.raw_footer, ['My Song', 'Author Unknown', 'My copyright', 'CCLI License: 1234'], + self.assertEqual(service_item.raw_footer, ['My Song', 'My copyright', 'CCLI License: 1234'], 'The array should be returned correctly with a song, an author, copyright and ccli') # WHEN: I amend the CCLI value @@ -126,5 +126,5 @@ class TestMediaItem(TestCase, TestMixin): self.media_item.generate_footer(service_item, mock_song) # THEN: I would get an amended footer string - self.assertEqual(service_item.raw_footer, ['My Song', 'Author Unknown', 'My copyright', 'CCLI License: 4321'], + self.assertEqual(service_item.raw_footer, ['My Song', 'My copyright', 'CCLI License: 4321'], 'The array should be returned correctly with a song, an author, copyright and amended ccli') From cec47fc950ae09cde0ce5680084c60fc0795cba9 Mon Sep 17 00:00:00 2001 From: Andreas Preikschat Date: Sun, 20 Apr 2014 14:06:02 +0200 Subject: [PATCH 22/28] fixed bug 1146964 Fixes: https://launchpad.net/bugs/1146964 --- openlp/core/ui/advancedtab.py | 2 +- openlp/core/ui/exceptionform.py | 2 +- openlp/core/ui/themeform.py | 2 +- openlp/plugins/images/lib/mediaitem.py | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/openlp/core/ui/advancedtab.py b/openlp/core/ui/advancedtab.py index b2c8cd14b..a5ce09bdf 100644 --- a/openlp/core/ui/advancedtab.py +++ b/openlp/core/ui/advancedtab.py @@ -511,7 +511,7 @@ class AdvancedTab(SettingsTab): """ Select an image for the default display screen. """ - file_filters = '%s;;%s (*.*) (*)' % (get_images_filter(), UiStrings().AllFiles) + file_filters = '%s;;%s (*.*)' % (get_images_filter(), UiStrings().AllFiles) filename = QtGui.QFileDialog.getOpenFileName(self, translate('OpenLP.AdvancedTab', 'Open File'), '', file_filters) if filename: diff --git a/openlp/core/ui/exceptionform.py b/openlp/core/ui/exceptionform.py index ae3bc9db0..e0228a43b 100644 --- a/openlp/core/ui/exceptionform.py +++ b/openlp/core/ui/exceptionform.py @@ -228,7 +228,7 @@ class ExceptionForm(QtGui.QDialog, Ui_ExceptionDialog, RegistryProperties): """ files = QtGui.QFileDialog.getOpenFileName(self, translate('ImagePlugin.ExceptionDialog', 'Select Attachment'), Settings().value(self.settings_section + '/last directory'), - '%s (*.*) (*)' % UiStrings().AllFiles) + '%s (*)' % UiStrings().AllFiles) log.info('New files(s) %s', str(files)) if files: self.file_attachment = str(files) diff --git a/openlp/core/ui/themeform.py b/openlp/core/ui/themeform.py index fbfc1035c..d9b61e117 100644 --- a/openlp/core/ui/themeform.py +++ b/openlp/core/ui/themeform.py @@ -432,7 +432,7 @@ class ThemeForm(QtGui.QWizard, Ui_ThemeWizard, RegistryProperties): Background Image button pushed. """ images_filter = get_images_filter() - images_filter = '%s;;%s (*.*) (*)' % (images_filter, UiStrings().AllFiles) + images_filter = '%s;;%s (*.*)' % (images_filter, UiStrings().AllFiles) filename = QtGui.QFileDialog.getOpenFileName(self, translate('OpenLP.ThemeWizard', 'Select Image'), '', images_filter) if filename: diff --git a/openlp/plugins/images/lib/mediaitem.py b/openlp/plugins/images/lib/mediaitem.py index d24a18434..c28f1e834 100644 --- a/openlp/plugins/images/lib/mediaitem.py +++ b/openlp/plugins/images/lib/mediaitem.py @@ -75,7 +75,7 @@ class ImageMediaItem(MediaManagerItem): def retranslateUi(self): self.on_new_prompt = translate('ImagePlugin.MediaItem', 'Select Image(s)') file_formats = get_images_filter() - self.on_new_file_masks = '%s;;%s (*.*) (*)' % (file_formats, UiStrings().AllFiles) + self.on_new_file_masks = '%s;;%s (*)' % (file_formats, UiStrings().AllFiles) self.add_group_action.setText(UiStrings().AddGroup) self.add_group_action.setToolTip(UiStrings().AddGroup) self.replace_action.setText(UiStrings().ReplaceBG) From c691fc55d0f0d287cbf24e96f6a5b13ef32017a6 Mon Sep 17 00:00:00 2001 From: Andreas Preikschat Date: Sun, 20 Apr 2014 15:00:14 +0200 Subject: [PATCH 23/28] added a test --- tests/functional/test_init.py | 70 +++++++++++++++++++++++++++++++++++ 1 file changed, 70 insertions(+) create mode 100644 tests/functional/test_init.py diff --git a/tests/functional/test_init.py b/tests/functional/test_init.py new file mode 100644 index 000000000..6fc7d7f70 --- /dev/null +++ b/tests/functional/test_init.py @@ -0,0 +1,70 @@ +# -*- coding: utf-8 -*- +# vim: autoindent shiftwidth=4 expandtab textwidth=120 tabstop=4 softtabstop=4 + +############################################################################### +# OpenLP - Open Source Lyrics Projection # +# --------------------------------------------------------------------------- # +# Copyright (c) 2008-2014 Raoul Snyman # +# Portions copyright (c) 2008-2014 Tim Bentley, Gerald Britton, Jonathan # +# Corwin, Samuel Findlay, Michael Gorven, Scott Guerrieri, Matthias Hub, # +# Meinert Jordan, Armin Köhler, Erik Lundin, Edwin Lunando, Brian T. Meyer. # +# Joshua Miller, Stevan Pettit, Andreas Preikschat, Mattias Põldaru, # +# Christian Richter, Philip Ridout, Simon Scudder, Jeffrey Smith, # +# Maikel Stuivenberg, Martin Thompson, Jon Tibble, Dave Warnock, # +# Frode Woldsund, Martin Zibricky, Patrick Zimmermann # +# --------------------------------------------------------------------------- # +# 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.__init__ package. +""" +import os + +from unittest import TestCase +from unittest.mock import MagicMock, patch +from PyQt4 import QtCore + +from openlp.core import OpenLP +from tests.helpers.testmixin import TestMixin + + +TEST_PATH = os.path.abspath(os.path.join(os.path.dirname(__file__), '..', 'resources')) + + +class TestInit(TestCase, TestMixin): + def setUp(self): + with patch('openlp.core.common.OpenLPMixin.__init__') as constructor: + constructor.return_value = None + self.openlp = OpenLP(list()) + + def tearDown(self): + del self.openlp + + def event_test(self): + """ + Test the reimplemented event method + """ + # GIVEN: A file path and a QEvent. + file_path = os.path.join(TEST_PATH, 'church.jpg') + mocked_file_method = MagicMock(return_value=file_path) + event = QtCore.QEvent(QtCore.QEvent.FileOpen) + event.file = mocked_file_method + + # WHEN: Call the vent method. + result = self.openlp.event(event) + + # THEN: The path should be inserted. + self.assertTrue(result, "The method should have returned True.") + mocked_file_method.assert_called_once_with() + self.assertEqual(self.openlp.args[0], file_path, "The path should be in args.") From dee8ed285035e77c56abfada996f635bc795f861 Mon Sep 17 00:00:00 2001 From: Raoul Snyman Date: Mon, 21 Apr 2014 00:23:26 +0200 Subject: [PATCH 24/28] Fix an exception raised when loading media players if the configuration is blank --- openlp/core/ui/media/__init__.py | 2 +- tests/functional/openlp_core_ui/test_media.py | 128 ++++++++++++++++++ 2 files changed, 129 insertions(+), 1 deletion(-) create mode 100644 tests/functional/openlp_core_ui/test_media.py diff --git a/openlp/core/ui/media/__init__.py b/openlp/core/ui/media/__init__.py index 8cc10b6a1..b2b6ab0b8 100644 --- a/openlp/core/ui/media/__init__.py +++ b/openlp/core/ui/media/__init__.py @@ -90,7 +90,7 @@ def get_media_players(): overridden_player = 'auto' else: overridden_player = '' - saved_players_list = saved_players.replace('[', '').replace(']', '').split(',') + saved_players_list = saved_players.replace('[', '').replace(']', '').split(',') if saved_players else [] return saved_players_list, overridden_player diff --git a/tests/functional/openlp_core_ui/test_media.py b/tests/functional/openlp_core_ui/test_media.py new file mode 100644 index 000000000..d59690949 --- /dev/null +++ b/tests/functional/openlp_core_ui/test_media.py @@ -0,0 +1,128 @@ +# -*- coding: utf-8 -*- +# vim: autoindent shiftwidth=4 expandtab textwidth=120 tabstop=4 softtabstop=4 + +############################################################################### +# OpenLP - Open Source Lyrics Projection # +# --------------------------------------------------------------------------- # +# Copyright (c) 2008-2014 Raoul Snyman # +# Portions copyright (c) 2008-2014 Tim Bentley, Gerald Britton, Jonathan # +# Corwin, Samuel Findlay, Michael Gorven, Scott Guerrieri, Matthias Hub, # +# Meinert Jordan, Armin Köhler, Erik Lundin, Edwin Lunando, Brian T. Meyer. # +# Joshua Miller, Stevan Pettit, Andreas Preikschat, Mattias Põldaru, # +# Christian Richter, Philip Ridout, Simon Scudder, Jeffrey Smith, # +# Maikel Stuivenberg, Martin Thompson, Jon Tibble, Dave Warnock, # +# Frode Woldsund, Martin Zibricky, Patrick Zimmermann # +# --------------------------------------------------------------------------- # +# 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.ui package. +""" +from PyQt4 import QtCore +from unittest import TestCase + +from openlp.core.ui.media import get_media_players + +from tests.functional import MagicMock, patch +from tests.helpers.testmixin import TestMixin + + +class TestMedia(TestCase, TestMixin): + + def setUp(self): + pass + + def test_get_media_players_no_config(self): + """ + Test that when there's no config, get_media_players() returns an empty list of players (not a string) + """ + def value_results(key): + if key == 'media/players': + return '' + else: + return False + + # GIVEN: A mocked out Settings() object + with patch('openlp.core.ui.media.Settings.value') as mocked_value: + mocked_value.side_effect = value_results + + # WHEN: get_media_players() is called + used_players, overridden_player = get_media_players() + + # THEN: the used_players should be an empty list, and the overridden player should be an empty string + self.assertEqual([], used_players, 'Used players should be an empty list') + self.assertEqual('', overridden_player, 'Overridden player should be an empty string') + + def test_get_media_players_no_players(self): + """ + Test that when there's no players but overridden player is set, get_media_players() returns 'auto' + """ + def value_results(key): + if key == 'media/override player': + return QtCore.Qt.Checked + else: + return '' + + # GIVEN: A mocked out Settings() object + with patch('openlp.core.ui.media.Settings.value') as mocked_value: + mocked_value.side_effect = value_results + + # WHEN: get_media_players() is called + used_players, overridden_player = get_media_players() + + # THEN: the used_players should be an empty list, and the overridden player should be an empty string + self.assertEqual([], used_players, 'Used players should be an empty list') + self.assertEqual('auto', overridden_player, 'Overridden player should be "auto"') + + def test_get_media_players_with_valid_list(self): + """ + Test that when get_media_players() is called the string list is interpreted correctly + """ + def value_results(key): + if key == 'media/players': + return '[vlc,webkit,phonon]' + else: + return False + + # GIVEN: A mocked out Settings() object + with patch('openlp.core.ui.media.Settings.value') as mocked_value: + mocked_value.side_effect = value_results + + # WHEN: get_media_players() is called + used_players, overridden_player = get_media_players() + + # THEN: the used_players should be an empty list, and the overridden player should be an empty string + self.assertEqual(['vlc', 'webkit', 'phonon'], used_players, 'Used players should be correct') + self.assertEqual('', overridden_player, 'Overridden player should be an empty string') + + def test_get_media_players_with_overridden_player(self): + """ + Test that when get_media_players() is called the overridden player is correctly set + """ + def value_results(key): + if key == 'media/players': + return '[vlc,webkit,phonon]' + else: + return QtCore.Qt.Checked + + # GIVEN: A mocked out Settings() object + with patch('openlp.core.ui.media.Settings.value') as mocked_value: + mocked_value.side_effect = value_results + + # WHEN: get_media_players() is called + used_players, overridden_player = get_media_players() + + # THEN: the used_players should be an empty list, and the overridden player should be an empty string + self.assertEqual(['vlc', 'webkit', 'phonon'], used_players, 'Used players should be correct') + self.assertEqual('vlc,webkit,phonon', overridden_player, 'Overridden player should be a string of players') \ No newline at end of file From ac44396a405b57d7650b135ebe21d27b51995852 Mon Sep 17 00:00:00 2001 From: Samuel Mehrbrodt Date: Mon, 21 Apr 2014 11:58:09 +0200 Subject: [PATCH 25/28] 2 row layout for author add widgets --- openlp/plugins/songs/forms/editsongdialog.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/openlp/plugins/songs/forms/editsongdialog.py b/openlp/plugins/songs/forms/editsongdialog.py index 7a491032b..7935b892a 100644 --- a/openlp/plugins/songs/forms/editsongdialog.py +++ b/openlp/plugins/songs/forms/editsongdialog.py @@ -118,18 +118,21 @@ class Ui_EditSongDialog(object): self.authors_group_box.setObjectName('authors_group_box') self.authors_layout = QtGui.QVBoxLayout(self.authors_group_box) self.authors_layout.setObjectName('authors_layout') - self.author_add_layout = QtGui.QHBoxLayout() + self.author_add_layout = QtGui.QVBoxLayout() self.author_add_layout.setObjectName('author_add_layout') + self.author_type_layout = QtGui.QHBoxLayout() + self.author_type_layout.setObjectName('author_type_layout') self.authors_combo_box = create_combo_box(self.authors_group_box, 'authors_combo_box') self.author_add_layout.addWidget(self.authors_combo_box) self.author_types_combo_box = create_combo_box(self.authors_group_box, 'author_types_combo_box', editable=False) # Need to give these boxes some min width, else they are too small self.authors_combo_box.setMinimumWidth(150) self.author_types_combo_box.setMinimumWidth(80) - self.author_add_layout.addWidget(self.author_types_combo_box) + self.author_type_layout.addWidget(self.author_types_combo_box) self.author_add_button = QtGui.QPushButton(self.authors_group_box) self.author_add_button.setObjectName('author_add_button') - self.author_add_layout.addWidget(self.author_add_button) + self.author_type_layout.addWidget(self.author_add_button) + self.author_add_layout.addLayout(self.author_type_layout) self.authors_layout.addLayout(self.author_add_layout) self.authors_list_view = QtGui.QListWidget(self.authors_group_box) self.authors_list_view.setAlternatingRowColors(True) From 70a5795cd249f2621e1c3b67b91ec0a8f9ce2ede Mon Sep 17 00:00:00 2001 From: Samuel Mehrbrodt Date: Mon, 21 Apr 2014 11:58:53 +0200 Subject: [PATCH 26/28] Remove min width --- openlp/plugins/songs/forms/editsongdialog.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/openlp/plugins/songs/forms/editsongdialog.py b/openlp/plugins/songs/forms/editsongdialog.py index 7935b892a..d0fb51a2d 100644 --- a/openlp/plugins/songs/forms/editsongdialog.py +++ b/openlp/plugins/songs/forms/editsongdialog.py @@ -125,9 +125,6 @@ class Ui_EditSongDialog(object): self.authors_combo_box = create_combo_box(self.authors_group_box, 'authors_combo_box') self.author_add_layout.addWidget(self.authors_combo_box) self.author_types_combo_box = create_combo_box(self.authors_group_box, 'author_types_combo_box', editable=False) - # Need to give these boxes some min width, else they are too small - self.authors_combo_box.setMinimumWidth(150) - self.author_types_combo_box.setMinimumWidth(80) self.author_type_layout.addWidget(self.author_types_combo_box) self.author_add_button = QtGui.QPushButton(self.authors_group_box) self.author_add_button.setObjectName('author_add_button') From cc0af8845ec8bc80e73d974706e6fb41477fabfb Mon Sep 17 00:00:00 2001 From: Samuel Mehrbrodt Date: Mon, 21 Apr 2014 12:06:17 +0200 Subject: [PATCH 27/28] PEP8 --- openlp/plugins/songs/lib/upgrade.py | 3 ++- tests/functional/openlp_plugins/songs/test_mediaitem.py | 6 +++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/openlp/plugins/songs/lib/upgrade.py b/openlp/plugins/songs/lib/upgrade.py index 59355bbec..580ae767d 100644 --- a/openlp/plugins/songs/lib/upgrade.py +++ b/openlp/plugins/songs/lib/upgrade.py @@ -112,7 +112,8 @@ def upgrade_4(session, metadata): op.create_table('authors_songs_tmp', Column('author_id', types.Integer(), ForeignKey('authors.id'), primary_key=True), Column('song_id', types.Integer(), ForeignKey('songs.id'), primary_key=True), - Column('author_type', types.String(), primary_key=True, nullable=False, server_default=text('""'))) + Column('author_type', types.String(), primary_key=True, + nullable=False, server_default=text('""'))) op.execute('INSERT INTO authors_songs_tmp SELECT author_id, song_id, "" FROM authors_songs') op.drop_table('authors_songs') op.rename_table('authors_songs_tmp', 'authors_songs') diff --git a/tests/functional/openlp_plugins/songs/test_mediaitem.py b/tests/functional/openlp_plugins/songs/test_mediaitem.py index 1b372aa20..308881c2e 100644 --- a/tests/functional/openlp_plugins/songs/test_mediaitem.py +++ b/tests/functional/openlp_plugins/songs/test_mediaitem.py @@ -76,19 +76,19 @@ class TestMediaItem(TestCase, TestMixin): mock_author.display_name = 'my author' mock_author_song = MagicMock() mock_author_song.author = mock_author - mock_author_song.author_type=AuthorType.Music + mock_author_song.author_type = AuthorType.Music mock_song.authors_songs.append(mock_author_song) mock_author = MagicMock() mock_author.display_name = 'another author' mock_author_song = MagicMock() mock_author_song.author = mock_author - mock_author_song.author_type=AuthorType.Words + mock_author_song.author_type = AuthorType.Words mock_song.authors_songs.append(mock_author_song) mock_author = MagicMock() mock_author.display_name = 'translator' mock_author_song = MagicMock() mock_author_song.author = mock_author - mock_author_song.author_type=AuthorType.Translation + mock_author_song.author_type = AuthorType.Translation mock_song.authors_songs.append(mock_author_song) mock_song.copyright = 'My copyright' service_item = ServiceItem(None) From 6eccf3eadf3d0048319dcf70f2fd407a27ee383c Mon Sep 17 00:00:00 2001 From: Samuel Mehrbrodt Date: Mon, 21 Apr 2014 16:40:54 +0200 Subject: [PATCH 28/28] change display of list view --- openlp/plugins/songs/lib/db.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openlp/plugins/songs/lib/db.py b/openlp/plugins/songs/lib/db.py index fa53bccc8..91649c951 100644 --- a/openlp/plugins/songs/lib/db.py +++ b/openlp/plugins/songs/lib/db.py @@ -48,7 +48,7 @@ class Author(BaseModel): """ def get_display_name(self, author_type=None): if author_type: - return "%s: %s" % (AuthorType.Types[author_type], self.display_name) + return "%s (%s)" % (self.display_name, AuthorType.Types[author_type]) return self.display_name