← Back to team overview

openlp-core team mailing list archive

[Merge] lp:~sam92/openlp/author-types into lp:openlp

 

Samuel Mehrbrodt has proposed merging lp:~sam92/openlp/author-types into lp:openlp.

Requested reviews:
  Tim Bentley (trb143)
  Andreas Preikschat (googol)
  Raoul Snyman (raoul-snyman)

For more details, see:
https://code.launchpad.net/~sam92/openlp/author-types/+merge/216592

Add support for Author Type

lp:~sam92/openlp/author-types (revision 2368)
[SUCCESS] http://ci.openlp.org/job/Branch-01-Pull/377/
[SUCCESS] http://ci.openlp.org/job/Branch-02-Functional-Tests/334/
[SUCCESS] http://ci.openlp.org/job/Branch-03-Interface-Tests/281/
[SUCCESS] http://ci.openlp.org/job/Branch-04-Windows_Tests/243/
[SUCCESS] http://ci.openlp.org/job/Branch-05a-Code_Analysis/167/
[SUCCESS] http://ci.openlp.org/job/Branch-05b-Test_Coverage/42/
-- 
https://code.launchpad.net/~sam92/openlp/author-types/+merge/216592
Your team OpenLP Core is subscribed to branch lp:openlp.
=== modified file '.bzrignore'
--- .bzrignore	2014-04-08 13:46:08 +0000
+++ .bzrignore	2014-04-21 14:44:38 +0000
@@ -6,6 +6,8 @@
 *.ropeproject
 *.e4*
 .eric4project
+.komodotools
+*.komodoproject
 list
 openlp.org 2.0.e4*
 documentation/build/html

=== modified file 'openlp/core/lib/db.py'
--- openlp/core/lib/db.py	2014-03-30 11:02:25 +0000
+++ openlp/core/lib/db.py	2014-04-21 14:44:38 +0000
@@ -194,6 +194,7 @@
                 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 @@
         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:

=== modified file 'openlp/core/ui/firsttimeform.py'
--- openlp/core/ui/firsttimeform.py	2014-04-12 18:21:15 +0000
+++ openlp/core/ui/firsttimeform.py	2014-04-21 14:44:38 +0000
@@ -114,10 +114,10 @@
         """
         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.
         """

=== modified file 'openlp/core/ui/themeform.py'
--- openlp/core/ui/themeform.py	2014-04-20 12:06:02 +0000
+++ openlp/core/ui/themeform.py	2014-04-21 14:44:38 +0000
@@ -90,7 +90,7 @@
         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 @@
         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)

=== modified file 'openlp/core/ui/wizard.py'
--- openlp/core/ui/wizard.py	2014-03-20 19:10:31 +0000
+++ openlp/core/ui/wizard.py	2014-04-21 14:44:38 +0000
@@ -197,7 +197,7 @@
         """
         Run the wizard.
         """
-        self.setDefaults()
+        self.set_defaults()
         return QtGui.QWizard.exec_(self)
 
     def reject(self):

=== modified file 'openlp/plugins/bibles/forms/bibleimportform.py'
--- openlp/plugins/bibles/forms/bibleimportform.py	2014-03-21 18:23:35 +0000
+++ openlp/plugins/bibles/forms/bibleimportform.py	2014-04-21 14:44:38 +0000
@@ -465,7 +465,7 @@
         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.
         """

=== modified file 'openlp/plugins/bibles/forms/bibleupgradeform.py'
--- openlp/plugins/bibles/forms/bibleupgradeform.py	2014-04-01 17:07:25 +0000
+++ openlp/plugins/bibles/forms/bibleupgradeform.py	2014-04-21 14:44:38 +0000
@@ -307,7 +307,7 @@
         if self.currentPage() == self.progress_page:
             return True
 
-    def setDefaults(self):
+    def set_defaults(self):
         """
         Set default values for the wizard pages.
         """

=== modified file 'openlp/plugins/songs/forms/duplicatesongremovalform.py'
--- openlp/plugins/songs/forms/duplicatesongremovalform.py	2014-04-14 18:53:33 +0000
+++ openlp/plugins/songs/forms/duplicatesongremovalform.py	2014-04-21 14:44:38 +0000
@@ -264,7 +264,7 @@
         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.
         """

=== modified file 'openlp/plugins/songs/forms/editsongdialog.py'
--- openlp/plugins/songs/forms/editsongdialog.py	2014-03-17 19:05:55 +0000
+++ openlp/plugins/songs/forms/editsongdialog.py	2014-04-21 14:44:38 +0000
@@ -118,13 +118,18 @@
         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)
+        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)
@@ -330,7 +335,7 @@
             translate('SongsPlugin.EditSongForm', '<strong>Warning:</strong> 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 @@
     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

=== modified file 'openlp/plugins/songs/forms/editsongform.py'
--- openlp/plugins/songs/forms/editsongform.py	2014-04-12 20:19:22 +0000
+++ openlp/plugins/songs/forms/editsongform.py	2014-04-21 14:44:38 +0000
@@ -42,7 +42,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, 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
@@ -122,12 +122,12 @@
             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):
@@ -217,8 +217,8 @@
         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())
@@ -302,6 +302,15 @@
             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(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):
         """
         Load the topics into the combobox.
@@ -454,10 +463,8 @@
         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 +503,7 @@
         """
         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 +521,7 @@
                     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:
@@ -521,11 +529,11 @@
         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:
-                self._add_author_to_list(author)
+                self._add_author_to_list(author, author_type)
             self.authors_combo_box.setCurrentIndex(0)
         else:
             QtGui.QMessageBox.warning(
@@ -905,13 +913,13 @@
         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)

=== modified file 'openlp/plugins/songs/forms/songimportform.py'
--- openlp/plugins/songs/forms/songimportform.py	2014-03-21 21:38:08 +0000
+++ openlp/plugins/songs/forms/songimportform.py	2014-04-21 14:44:38 +0000
@@ -304,7 +304,7 @@
         """
         self.source_page.emit(QtCore.SIGNAL('completeChanged()'))
 
-    def setDefaults(self):
+    def set_defaults(self):
         """
         Set default form values for the song import wizard.
         """

=== modified file 'openlp/plugins/songs/lib/__init__.py'
--- openlp/plugins/songs/lib/__init__.py	2014-03-21 21:38:08 +0000
+++ openlp/plugins/songs/lib/__init__.py	2014-04-21 14:44:38 +0000
@@ -390,7 +390,7 @@
     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:
+    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:

=== modified file 'openlp/plugins/songs/lib/db.py'
--- openlp/plugins/songs/lib/db.py	2014-03-21 21:38:08 +0000
+++ openlp/plugins/songs/lib/db.py	2014-04-21 14:44:38 +0000
@@ -35,19 +35,52 @@
 
 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
+from openlp.core.lib import translate
 
 
 class Author(BaseModel):
     """
     Author model
     """
+    def get_display_name(self, author_type=None):
+        if author_type:
+            return "%s (%s)" % (self.display_name, AuthorType.Types[author_type])
+        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
 
 
+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')
+    }
+
+
 class Book(BaseModel):
     """
     Book model
@@ -67,6 +100,7 @@
     """
     Song model
     """
+
     def __init__(self):
         self.sort_key = []
 
@@ -120,6 +154,7 @@
 
         * author_id
         * song_id
+        * author_type
 
     **media_files Table**
         * id
@@ -230,7 +265,8 @@
     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, nullable=False, server_default=text('""'))
     )
 
     # Definition of the "songs_topics" table
@@ -241,10 +277,15 @@
     )
 
     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),
+        # 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),
         '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)

=== modified file 'openlp/plugins/songs/lib/mediaitem.py'
--- openlp/plugins/songs/lib/mediaitem.py	2014-04-14 19:46:02 +0000
+++ openlp/plugins/songs/lib/mediaitem.py	2014-04-21 14:44:38 +0000
@@ -44,7 +44,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, 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
 
@@ -234,8 +234,7 @@
             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 song.title
             song_name = QtGui.QListWidgetItem(song_detail)
             song_name.setData(QtCore.Qt.UserRole, song.id)
             self.list_view.addItem(song_name)
@@ -464,23 +463,53 @@
     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_words_music = []
+        authors_translation = []
+        authors_none = []
+        for author_song in song.authors_songs:
+            if author_song.author_type == AuthorType.Words:
+                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_music + 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_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)))
+        if 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)))
         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):
         """

=== modified file 'openlp/plugins/songs/lib/ui.py'
--- openlp/plugins/songs/lib/ui.py	2014-03-21 21:38:08 +0000
+++ openlp/plugins/songs/lib/ui.py	2014-04-21 14:44:38 +0000
@@ -40,7 +40,7 @@
     # 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')  # 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')

=== modified file 'openlp/plugins/songs/lib/upgrade.py'
--- openlp/plugins/songs/lib/upgrade.py	2014-03-30 11:02:25 +0000
+++ openlp/plugins/songs/lib/upgrade.py	2014-04-21 14:44:38 +0000
@@ -32,14 +32,14 @@
 """
 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
 
 from openlp.core.lib.db import get_upgrade_op
 
 log = logging.getLogger(__name__)
-__version__ = 3
+__version__ = 4
 
 
 def upgrade_1(session, metadata):
@@ -97,3 +97,25 @@
             op.add_column('songs', Column('temporary', types.Boolean(), server_default=false()))
     except OperationalError:
         log.info('Upgrade 3 has already been run')
+
+
+def upgrade_4(session, metadata):
+    """
+    Version 4 upgrade.
+
+    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.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')

=== modified file 'openlp/plugins/songs/lib/xml.py'
--- openlp/plugins/songs/lib/xml.py	2014-04-08 11:07:52 +0000
+++ openlp/plugins/songs/lib/xml.py	2014-04-21 14:44:38 +0000
@@ -71,7 +71,7 @@
 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, AuthorType, Book, Song, Topic
 from openlp.core.utils import get_application_version
 
 log = logging.getLogger(__name__)
@@ -166,7 +166,7 @@
     supported by the :class:`OpenLyrics` class:
 
     ``<authors>``
-        OpenLP does not support the attribute *type* and *lang*.
+        OpenLP does not support the attribute *lang*.
 
     ``<chord>``
         This property is not supported.
@@ -269,10 +269,18 @@
                 '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:
+                    # 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
@@ -501,16 +509,20 @@
         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):
         """

=== modified file 'tests/functional/openlp_plugins/songs/test_mediaitem.py'
--- tests/functional/openlp_plugins/songs/test_mediaitem.py	2014-03-14 22:08:44 +0000
+++ tests/functional/openlp_plugins/songs/test_mediaitem.py	2014-04-21 14:44:38 +0000
@@ -10,6 +10,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 tests.functional import patch, MagicMock
 from tests.helpers.testmixin import TestMixin
 
@@ -45,10 +46,12 @@
         # 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 @@
         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 @@
         # 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 @@
         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 @@
         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', '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 @@
         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', 'My copyright', 'CCLI License: 4321'],
                          'The array should be returned correctly with a song, an author, copyright and amended ccli')


Follow ups