openlp-core team mailing list archive

[Samuel Mehrbrodt <s.mehrbrodt@xxxxxxxxx>]

Samuel Mehrbrodt has proposed merging lp:~sam92/openlp/bug-1695620 into lp:openlp.

Commit message:
Make footer configurable

Requested reviews:
  Raoul Snyman (raoul-snyman)
  Tim Bentley (trb143)
  Tomas Groth (tomasgroth)
Related bugs:
  Bug #1694430 in OpenLP: "Allow to show alternate title in footer"
  https://bugs.launchpad.net/openlp/+bug/1694430
  Bug #1695620 in OpenLP: "Make footer elements customizable"
  https://bugs.launchpad.net/openlp/+bug/1695620

For more details, see:
https://code.launchpad.net/~sam92/openlp/bug-1695620/+merge/363518

Make footer configurable

- Make use of Mako for footer generation,  configurable in song settings
- removed now obsolete and via template better configurable options to display "songbook", "written by" and "copyright" information in footer
- added explanation box for so far used settings as Mako placeholders
- added songs configuration setting for template including reset button
- added default template replacing currently existing configuration as best as possible (should be backwards compatible or at least be adaptable to correspond to former settings)
- write and adapt tests for new and removed functionality
- Added some more available fields in the footer: Alternate Title, CCLI Number, Topics, Authors (all music, all words)
-- 
Your team OpenLP Core is subscribed to branch lp:openlp.

=== modified file 'openlp/core/lib/serviceitem.py'
--- openlp/core/lib/serviceitem.py	2019-02-14 15:09:09 +0000
+++ openlp/core/lib/serviceitem.py	2019-02-21 21:38:53 +0000
@@ -81,7 +81,8 @@
         self.items = []
         self.icon = UiIcons().default
         self.raw_footer = []
-        self.foot_text = ''
+        # Plugins can set footer_html themselves. If they don't, it will be generated from raw_footer.
+        self.footer_html = ''
         self.theme = None
         self.service_item_type = None
         self.unique_identifier = 0
@@ -165,7 +166,8 @@
         # the dict instead of rendering them again.
         previous_pages = {}
         index = 0
-        self.foot_text = '<br>'.join([_f for _f in self.raw_footer if _f])
+        if not self.footer_html:
+            self.footer_html = '<br>'.join([_f for _f in self.raw_footer if _f])
         for raw_slide in self.slides:
             verse_tag = raw_slide['verse']
             if verse_tag in previous_pages and previous_pages[verse_tag][0] == raw_slide:
@@ -178,7 +180,7 @@
                     'title': raw_slide['title'],
                     'text': render_tags(page),
                     'verse': index,
-                    'footer': self.foot_text,
+                    'footer': self.footer_html,
                 }
                 self._rendered_slides.append(rendered_slide)
                 display_slide = {

=== modified file 'openlp/core/ui/printserviceform.py'
--- openlp/core/ui/printserviceform.py	2019-02-14 15:09:09 +0000
+++ openlp/core/ui/printserviceform.py	2019-02-21 21:38:53 +0000
@@ -235,11 +235,11 @@
                 for slide in range(len(item.get_frames())):
                     self._add_element('li', item.get_frame_title(slide), ol)
             # add footer
-            foot_text = item.foot_text
-            foot_text = foot_text.partition('<br>')[2]
-            if foot_text:
-                foot_text = html.escape(foot_text.replace('<br>', '\n'))
-                self._add_element('div', foot_text.replace('\n', '<br>'), parent=div, class_id='itemFooter')
+            footer_html = item.footer_html
+            footer_html = footer_html.partition('<br>')[2]
+            if footer_html:
+                footer_html = html.escape(footer_html.replace('<br>', '\n'))
+                self._add_element('div', footer_html.replace('\n', '<br>'), parent=div, classId='itemFooter')
         # Add service items' notes.
         if self.notes_check_box.isChecked():
             if item.notes:

=== modified file 'openlp/plugins/songs/lib/mediaitem.py'
--- openlp/plugins/songs/lib/mediaitem.py	2019-02-14 15:09:09 +0000
+++ openlp/plugins/songs/lib/mediaitem.py	2019-02-21 21:38:53 +0000
@@ -21,6 +21,7 @@
 ###############################################################################
 import logging
 import os
+import mako
 
 from PyQt5 import QtCore, QtWidgets
 from sqlalchemy.sql import and_, or_
@@ -35,7 +36,7 @@
 from openlp.core.lib.mediamanageritem import MediaManagerItem
 from openlp.core.lib.plugin import PluginStatus
 from openlp.core.lib.serviceitem import ItemCapabilities
-from openlp.core.lib.ui import create_widget_action
+from openlp.core.lib.ui import create_widget_action, critical_error_message_box
 from openlp.core.ui.icons import UiIcons
 from openlp.plugins.songs.forms.editsongform import EditSongForm
 from openlp.plugins.songs.forms.songexportform import SongExportForm
@@ -131,9 +132,6 @@
         self.is_search_as_you_type_enabled = Settings().value('advanced/search as type')
         self.update_service_on_edit = Settings().value(self.settings_section + '/update service on edit')
         self.add_song_from_service = Settings().value(self.settings_section + '/add song from service')
-        self.display_songbook = Settings().value(self.settings_section + '/display songbook')
-        self.display_written_by_text = Settings().value(self.settings_section + '/display written by')
-        self.display_copyright_symbol = Settings().value(self.settings_section + '/display copyright symbol')
 
     def retranslate_ui(self):
         self.search_text_label.setText('{text}:'.format(text=UiStrings().Search))
@@ -677,12 +675,8 @@
         item.raw_footer = []
         item.raw_footer.append(song.title)
         if authors_none:
-            # If the setting for showing "Written by:" is enabled, show it before unspecified authors.
-            if Settings().value('songs/display written by'):
-                item.raw_footer.append("{text}: {authors}".format(text=translate('OpenLP.Ui', 'Written by'),
-                                                                  authors=create_separated_list(authors_none)))
-            else:
-                item.raw_footer.append("{authors}".format(authors=create_separated_list(authors_none)))
+            item.raw_footer.append("{text}: {authors}".format(text=translate('OpenLP.Ui', 'Written by'),
+                                                              authors=create_separated_list(authors_none)))
         if authors_words_music:
             item.raw_footer.append("{text}: {authors}".format(text=AuthorType.Types[AuthorType.WordsAndMusic],
                                                               authors=create_separated_list(authors_words_music)))
@@ -696,34 +690,44 @@
             item.raw_footer.append("{text}: {authors}".format(text=AuthorType.Types[AuthorType.Translation],
                                                               authors=create_separated_list(authors_translation)))
         if song.copyright:
-            if self.display_copyright_symbol:
-                item.raw_footer.append("{symbol} {song}".format(symbol=SongStrings.CopyrightSymbol,
-                                                                song=song.copyright))
-            else:
-                item.raw_footer.append(song.copyright)
-        if self.display_songbook and song.songbook_entries:
-            songbooks = [str(songbook_entry) for songbook_entry in song.songbook_entries]
+            item.raw_footer.append("{symbol} {song}".format(symbol=SongStrings.CopyrightSymbol,
+                                                            song=song.copyright))
+        songbooks = [str(songbook_entry) for songbook_entry in song.songbook_entries]
+        if song.songbook_entries:
             item.raw_footer.append(", ".join(songbooks))
         if Settings().value('core/ccli number'):
-            item.raw_footer.append(translate('SongsPlugin.MediaItem',
-                                             'CCLI License: ') + Settings().value('core/ccli number'))
-        item.metadata.append('<em>{label}:</em> {title}'.format(label=translate('SongsPlugin.MediaItem', 'Title'),
-                                                                title=song.title))
-        if song.alternate_title:
-            item.metadata.append('<em>{label}:</em> {title}'.
-                                 format(label=translate('SongsPlugin.MediaItem', 'Alt Title'),
-                                        title=song.alternate_title))
-        if song.songbook_entries:
-            for songbook_entry in song.songbook_entries:
-                item.metadata.append('<em>{label}:</em> {book}/{num}/{pub}'.
-                                     format(label=translate('SongsPlugin.MediaItem', 'Songbook'),
-                                            book=songbook_entry.songbook.name,
-                                            num=songbook_entry.entry,
-                                            pub=songbook_entry.songbook.publisher))
-        if song.topics:
-            for topics in song.topics:
-                item.metadata.append('<em>{label}:</em> {topic}'.
-                                     format(label=translate('SongsPlugin.MediaItem', 'Topic'), topic=topics.name))
+            item.raw_footer.append(translate('SongsPlugin.MediaItem', 'CCLI License: ') +
+                                   Settings().value('core/ccli number'))
+        footer_template = Settings().value('songs/footer template')
+        # Keep this in sync with the list in songstab.py
+        vars = {
+            'title': song.title,
+            'alternate_title': song.alternate_title,
+            'authors_none_label': translate('OpenLP.Ui', 'Written by'),
+            'authors_none': authors_none,
+            'authors_words_label': AuthorType.Types[AuthorType.Words],
+            'authors_words': authors_words,
+            'authors_music_label': AuthorType.Types[AuthorType.Music],
+            'authors_music': authors_music,
+            'authors_words_music_label': AuthorType.Types[AuthorType.WordsAndMusic],
+            'authors_words_music': authors_words_music,
+            'authors_translation_label': AuthorType.Types[AuthorType.Translation],
+            'authors_translation': authors_translation,
+            'authors_words_all': authors_words + authors_words_music,
+            'authors_music_all': authors_music + authors_words_music,
+            'copyright': song.copyright,
+            'songbook_entries': songbooks,
+            'ccli_license': Settings().value('core/ccli number'),
+            'ccli_license_label': translate('SongsPlugin.MediaItem', 'CCLI License'),
+            'ccli_number': song.ccli_number,
+            'topics': [topic.name for topic in song.topics]
+        }
+        try:
+            item.footer_html = mako.template.Template(footer_template).render_unicode(**vars).replace('\n', '')
+        except mako.exceptions.SyntaxException:
+            log.error('Failed to render Song footer html:\n' + mako.exceptions.text_error_template().render())
+            critical_error_message_box(message=translate('SongsPlugin.MediaItem',
+                                                         'Failed to render Song footer html.\nSee log for details'))
         return authors_all
 
     def service_load(self, item):

=== modified file 'openlp/plugins/songs/lib/songstab.py'
--- openlp/plugins/songs/lib/songstab.py	2019-02-14 15:09:09 +0000
+++ openlp/plugins/songs/lib/songstab.py	2019-02-21 21:38:53 +0000
@@ -25,7 +25,7 @@
 from openlp.core.common.i18n import translate
 from openlp.core.common.settings import Settings
 from openlp.core.lib.settingstab import SettingsTab
-from openlp.plugins.songs.lib.ui import SongStrings
+from openlp.plugins.songs.lib.db import AuthorType
 
 
 class SongsTab(SettingsTab):
@@ -54,15 +54,6 @@
         self.songbook_slide_check_box = QtWidgets.QCheckBox(self.mode_group_box)
         self.songbook_slide_check_box.setObjectName('songbook_slide_check_box')
         self.mode_layout.addWidget(self.songbook_slide_check_box)
-        self.display_songbook_check_box = QtWidgets.QCheckBox(self.mode_group_box)
-        self.display_songbook_check_box.setObjectName('songbook_check_box')
-        self.mode_layout.addWidget(self.display_songbook_check_box)
-        self.display_written_by_check_box = QtWidgets.QCheckBox(self.mode_group_box)
-        self.display_written_by_check_box.setObjectName('written_by_check_box')
-        self.mode_layout.addWidget(self.display_written_by_check_box)
-        self.display_copyright_check_box = QtWidgets.QCheckBox(self.mode_group_box)
-        self.display_copyright_check_box.setObjectName('copyright_check_box')
-        self.mode_layout.addWidget(self.display_copyright_check_box)
         self.left_layout.addWidget(self.mode_group_box)
         # Chords group box
         self.chords_group_box = QtWidgets.QGroupBox(self.left_column)
@@ -93,20 +84,34 @@
         self.neolatin_notation_radio_button.setObjectName('neolatin_notation_radio_button')
         self.chords_layout.addWidget(self.neolatin_notation_radio_button)
         self.left_layout.addWidget(self.chords_group_box)
+        # Footer group box
+        self.footer_group_box = QtWidgets.QGroupBox(self.left_column)
+        self.footer_group_box.setObjectName('footer_group_box')
+        self.footer_layout = QtWidgets.QVBoxLayout(self.footer_group_box)
+        self.footer_layout.setObjectName('chords_layout')
+        self.footer_info_label = QtWidgets.QLabel(self.footer_group_box)
+        self.footer_layout.addWidget(self.footer_info_label)
+        self.footer_placeholder_info = QtWidgets.QTextEdit(self.footer_group_box)
+        self.footer_layout.addWidget(self.footer_placeholder_info)
+        self.footer_desc_label = QtWidgets.QLabel(self.footer_group_box)
+        self.footer_layout.addWidget(self.footer_desc_label)
+        self.footer_edit_box = QtWidgets.QTextEdit(self.footer_group_box)
+        self.footer_layout.addWidget(self.footer_edit_box)
+        self.footer_reset_button = QtWidgets.QPushButton(self.footer_group_box)
+        self.footer_layout.addWidget(self.footer_reset_button, alignment=QtCore.Qt.AlignRight)
+        self.right_layout.addWidget(self.footer_group_box)
         self.left_layout.addStretch()
         self.right_layout.addStretch()
         self.tool_bar_active_check_box.stateChanged.connect(self.on_tool_bar_active_check_box_changed)
         self.update_on_edit_check_box.stateChanged.connect(self.on_update_on_edit_check_box_changed)
         self.add_from_service_check_box.stateChanged.connect(self.on_add_from_service_check_box_changed)
         self.songbook_slide_check_box.stateChanged.connect(self.on_songbook_slide_check_box_changed)
-        self.display_songbook_check_box.stateChanged.connect(self.on_songbook_check_box_changed)
-        self.display_written_by_check_box.stateChanged.connect(self.on_written_by_check_box_changed)
-        self.display_copyright_check_box.stateChanged.connect(self.on_copyright_check_box_changed)
         self.mainview_chords_check_box.stateChanged.connect(self.on_mainview_chords_check_box_changed)
         self.disable_chords_import_check_box.stateChanged.connect(self.on_disable_chords_import_check_box_changed)
         self.english_notation_radio_button.clicked.connect(self.on_english_notation_button_clicked)
         self.german_notation_radio_button.clicked.connect(self.on_german_notation_button_clicked)
         self.neolatin_notation_radio_button.clicked.connect(self.on_neolatin_notation_button_clicked)
+        self.footer_reset_button.clicked.connect(self.on_footer_reset_button_clicked)
 
     def retranslate_ui(self):
         self.mode_group_box.setTitle(translate('SongsPlugin.SongsTab', 'Song related settings'))
@@ -117,12 +122,6 @@
                                                           'Import missing songs from Service files'))
         self.songbook_slide_check_box.setText(translate('SongsPlugin.SongsTab',
                                                         'Add Songbooks as first side'))
-        self.display_songbook_check_box.setText(translate('SongsPlugin.SongsTab', 'Display songbook in footer'))
-        self.display_written_by_check_box.setText(translate(
-            'SongsPlugin.SongsTab', 'Show "Written by:" in footer for unspecified authors'))
-        self.display_copyright_check_box.setText(translate('SongsPlugin.SongsTab',
-                                                           'Display "{symbol}" symbol before copyright '
-                                                           'info').format(symbol=SongStrings.CopyrightSymbol))
         self.chords_info_label.setText(translate('SongsPlugin.SongsTab', 'If enabled all text between "[" and "]" will '
                                                                          'be regarded as chords.'))
         self.chords_group_box.setTitle(translate('SongsPlugin.SongsTab', 'Chords'))
@@ -134,6 +133,53 @@
         self.german_notation_radio_button.setText(translate('SongsPlugin.SongsTab', 'German') + ' (C-D-E-F-G-A-H)')
         self.neolatin_notation_radio_button.setText(
             translate('SongsPlugin.SongsTab', 'Neo-Latin') + ' (Do-Re-Mi-Fa-Sol-La-Si)')
+        self.footer_group_box.setTitle(translate('SongsPlugin.SongsTab', 'Footer'))
+        # Keep this in sync with the list in mediaitem.py
+        const = '<code>"{}"</code>'
+        placeholders = [
+            # placeholder, description, can be empty, is a list
+            ['title', translate('SongsPlugin.SongsTab', 'Song Title'), False, False],
+            ['alternate_title', translate('SongsPlugin.SongsTab', 'Alternate Title'), True, False],
+            ['written_by', const.format(translate('SongsPlugin.SongsTab', 'Written By')), True, False],
+            ['authors_none', translate('SongsPlugin.SongsTab', 'Authors when type is not set'), False, True],
+            ['authors_words_label', const.format(AuthorType.Types[AuthorType.Words]), False, False],
+            ['authors_words', translate('SongsPlugin.SongsTab', 'Authors (Type "Words")'), False, True],
+            ['authors_music_label', const.format(AuthorType.Types[AuthorType.Music]), False, False],
+            ['authors_music', translate('SongsPlugin.SongsTab', 'Authors (Type "Music")'), False, True],
+            ['authors_words_music_label', const.format(AuthorType.Types[AuthorType.WordsAndMusic]), False, False],
+            ['authors_words_music', translate('SongsPlugin.SongsTab', 'Authors (Type "Words and Music")'), False, True],
+            ['authors_translation_label', const.format(AuthorType.Types[AuthorType.Translation]), False, False],
+            ['authors_translation', translate('SongsPlugin.SongsTab', 'Authors (Type "Translation")'), False, True],
+            ['authors_words_all', translate('SongsPlugin.SongsTab', 'Authors (Type "Words" & "Words and Music")'),
+             False, True],
+            ['authors_music_all', translate('SongsPlugin.SongsTab', 'Authors (Type "Music" & "Words and Music")'),
+             False, True],
+            ['copyright', translate('SongsPlugin.SongsTab', 'Copyright information'), True, False],
+            ['songbook_entries', translate('SongsPlugin.SongsTab', 'Songbook Entries'), False, True],
+            ['ccli_license', translate('SongsPlugin.SongsTab', 'CCLI License'), True, False],
+            ['ccli_license_label', const.format(translate('SongsPlugin.SongsTab', 'CCLI License')), False, False],
+            ['ccli_number', translate('SongsPlugin.SongsTab', 'Song CCLI Number'), True, False],
+            ['topics', translate('SongsPlugin.SongsTab', 'Topics'), False, True],
+        ]
+        placeholder_info = '<table style="background: #eee">\n<tr><th><b>{ph}</b></th><th><b>{desc}</b></th></tr>\n'\
+            .format(ph=translate('SongsPlugin.SongsTab', 'Placeholder'),
+                    desc=translate('SongsPlugin.SongsTab', 'Description'))
+        for placeholder in placeholders:
+            placeholder_info += '<tr><td>${{{pl}}}</td><td>{des}{opt}</td></tr>\n'\
+                                .format(pl=placeholder[0], des=placeholder[1],
+                                        opt=('&nbsp;¹' if placeholder[2] else '') +
+                                            ('&nbsp;²' if placeholder[3] else ''))
+        placeholder_info += '</table>'
+        placeholder_info += '\n<br/>¹ {}'.format(translate('SongsPlugin.SongsTab', 'can be empty'))
+        placeholder_info += '\n<br/>² {}'.format(translate('SongsPlugin.SongsTab', 'list of entries, can be empty'))
+        self.footer_placeholder_info.setHtml(placeholder_info)
+        self.footer_placeholder_info.setReadOnly(True)
+
+        self.footer_info_label.setText(translate('SongsPlugin.SongsTab', 'How to use Footers:'))
+        self.footer_desc_label.setText('{} (<a href="http://docs.makotemplates.org";>{}</a>):'
+                                       .format(translate('SongsPlugin.SongsTab', 'Footer Template'),
+                                               translate('SongsPlugin.SongsTab', 'Mako Syntax')))
+        self.footer_reset_button.setText(translate('SongsPlugin.SongsTab', 'Reset Template'))
 
     def on_search_as_type_check_box_changed(self, check_state):
         self.song_search = (check_state == QtCore.Qt.Checked)
@@ -150,15 +196,6 @@
     def on_songbook_slide_check_box_changed(self, check_state):
         self.songbook_slide = (check_state == QtCore.Qt.Checked)
 
-    def on_songbook_check_box_changed(self, check_state):
-        self.display_songbook = (check_state == QtCore.Qt.Checked)
-
-    def on_written_by_check_box_changed(self, check_state):
-        self.display_written_by = (check_state == QtCore.Qt.Checked)
-
-    def on_copyright_check_box_changed(self, check_state):
-        self.display_copyright_symbol = (check_state == QtCore.Qt.Checked)
-
     def on_mainview_chords_check_box_changed(self, check_state):
         self.mainview_chords = (check_state == QtCore.Qt.Checked)
 
@@ -174,6 +211,9 @@
     def on_neolatin_notation_button_clicked(self):
         self.chord_notation = 'neo-latin'
 
+    def on_footer_reset_button_clicked(self):
+        self.footer_edit_box.setPlainText(Settings().get_default_value('songs/footer template'))
+
     def load(self):
         settings = Settings()
         settings.beginGroup(self.settings_section)
@@ -181,9 +221,6 @@
         self.update_edit = settings.value('update service on edit')
         self.update_load = settings.value('add song from service')
         self.songbook_slide = settings.value('add songbook slide')
-        self.display_songbook = settings.value('display songbook')
-        self.display_written_by = settings.value('display written by')
-        self.display_copyright_symbol = settings.value('display copyright symbol')
         self.enable_chords = settings.value('enable chords')
         self.chord_notation = settings.value('chord notation')
         self.mainview_chords = settings.value('mainview chords')
@@ -191,9 +228,6 @@
         self.tool_bar_active_check_box.setChecked(self.tool_bar)
         self.update_on_edit_check_box.setChecked(self.update_edit)
         self.add_from_service_check_box.setChecked(self.update_load)
-        self.display_songbook_check_box.setChecked(self.display_songbook)
-        self.display_written_by_check_box.setChecked(self.display_written_by)
-        self.display_copyright_check_box.setChecked(self.display_copyright_symbol)
         self.chords_group_box.setChecked(self.enable_chords)
         self.mainview_chords_check_box.setChecked(self.mainview_chords)
         self.disable_chords_import_check_box.setChecked(self.disable_chords_import)
@@ -203,6 +237,7 @@
             self.neolatin_notation_radio_button.setChecked(True)
         else:
             self.english_notation_radio_button.setChecked(True)
+        self.footer_edit_box.setPlainText(settings.value('footer template'))
         settings.endGroup()
 
     def save(self):
@@ -211,13 +246,13 @@
         settings.setValue('display songbar', self.tool_bar)
         settings.setValue('update service on edit', self.update_edit)
         settings.setValue('add song from service', self.update_load)
-        settings.setValue('display songbook', self.display_songbook)
-        settings.setValue('display written by', self.display_written_by)
-        settings.setValue('display copyright symbol', self.display_copyright_symbol)
         settings.setValue('enable chords', self.chords_group_box.isChecked())
         settings.setValue('mainview chords', self.mainview_chords)
         settings.setValue('disable chords import', self.disable_chords_import)
         settings.setValue('chord notation', self.chord_notation)
+        # Only save footer template if it has been changed. This allows future updates
+        if self.footer_edit_box.toPlainText() != Settings().get_default_value('songs/footer template'):
+            settings.setValue('footer template', self.footer_edit_box.toPlainText())
         settings.setValue('add songbook slide', self.songbook_slide)
         settings.endGroup()
         if self.tab_visited:

=== modified file 'openlp/plugins/songs/songsplugin.py'
--- openlp/plugins/songs/songsplugin.py	2019-02-14 15:09:09 +0000
+++ openlp/plugins/songs/songsplugin.py	2019-02-21 21:38:53 +0000
@@ -66,11 +66,8 @@
     'songs/add song from service': True,
     'songs/add songbook slide': False,
     'songs/display songbar': True,
-    'songs/display songbook': False,
-    'songs/display written by': True,
-    'songs/display copyright symbol': False,
-    'songs/last directory import': None,
-    'songs/last directory export': None,
+    'songs/last directory import': '',
+    'songs/last directory export': '',
     'songs/songselect username': '',
     'songs/songselect password': '',
     'songs/songselect searches': '',
@@ -78,6 +75,59 @@
     'songs/chord notation': 'english',  # Can be english, german or neo-latin
     'songs/mainview chords': False,
     'songs/disable chords import': False,
+    'songs/footer template': """\
+${title}<br/>
+
+%if authors_none:
+  <%
+    authors = ", ".join(authors_none)
+  %>
+  ${authors_none_label}:&nbsp;${authors}<br/>
+%endif
+
+%if authors_words_music:
+  <%
+    authors = ", ".join(authors_words_music)
+  %>
+  ${authors_words_music_label}:&nbsp;${authors}<br/>
+%endif
+
+%if authors_words:
+  <%
+    authors = ", ".join(authors_words)
+  %>
+  ${authors_words_label}:&nbsp;${authors}<br/>
+%endif
+
+%if authors_music:
+  <%
+    authors = ", ".join(authors_music)
+  %>
+  ${authors_music_label}:&nbsp;${authors}<br/>
+%endif
+
+%if authors_translation:
+  <%
+    authors = ", ".join(authors_translation)
+  %>
+  ${authors_translation_label}:&nbsp;${authors}<br/>
+%endif
+
+%if copyright:
+  &copy;&nbsp;${copyright}<br/>
+%endif
+
+%if songbook_entries:
+  <%
+    entries = ", ".join(songbook_entries)
+  %>
+  ${entries}<br/>
+%endif
+
+%if ccli_license:
+  ${ccli_license_label}&nbsp;${ccli_license}<br/>
+%endif
+""",
 }
 
 

=== modified file 'tests/functional/openlp_plugins/songs/test_mediaitem.py'
--- tests/functional/openlp_plugins/songs/test_mediaitem.py	2019-02-14 15:09:09 +0000
+++ tests/functional/openlp_plugins/songs/test_mediaitem.py	2019-02-21 21:38:53 +0000
@@ -34,6 +34,62 @@
 from openlp.plugins.songs.lib.mediaitem import SongMediaItem
 from tests.helpers.testmixin import TestMixin
 
+__default_settings__ = {
+    'songs/footer template': """
+${title}<br/>
+
+%if authors_none:
+  <%
+    authors = ", ".join(authors_none)
+  %>
+  ${authors_none_label}:&nbsp;${authors}<br/>
+%endif
+
+%if authors_words_music:
+  <%
+    authors = ", ".join(authors_words_music)
+  %>
+  ${authors_words_music_label}:&nbsp;${authors}<br/>
+%endif
+
+%if authors_words:
+  <%
+    authors = ", ".join(authors_words)
+  %>
+  ${authors_words_label}:&nbsp;${authors}<br/>
+%endif
+
+%if authors_music:
+  <%
+    authors = ", ".join(authors_music)
+  %>
+  ${authors_music_label}:&nbsp;${authors}<br/>
+%endif
+
+%if authors_translation:
+  <%
+    authors = ", ".join(authors_translation)
+  %>
+  ${authors_translation_label}:&nbsp;${authors}<br/>
+%endif
+
+%if copyright:
+  &copy;&nbsp;${copyright}<br/>
+%endif
+
+%if songbook_entries:
+  <%
+    entries = ", ".join(songbook_entries)
+  %>
+  ${entries}<br/>
+%endif
+
+%if ccli_license:
+  ${ccli_license_label}&nbsp;${ccli_license}<br/>
+%endif
+"""
+}
+
 
 class TestMediaItem(TestCase, TestMixin):
     """
@@ -61,6 +117,7 @@
             self.media_item.display_copyright_symbol = False
         self.setup_application()
         self.build_settings()
+        Settings().extend_default_settings(__default_settings__)
         QtCore.QLocale.setDefault(QtCore.QLocale('en_GB'))
 
     def tearDown(self):
@@ -297,63 +354,45 @@
         """
         Test build songs footer with basic song and one author
         """
-        # GIVEN: A Song and a Service Item, mocked settings: True for 'songs/display written by'
-        # and False for 'core/ccli number' (ccli will cause traceback if true)
-
-        mocked_settings = MagicMock()
-        mocked_settings.value.side_effect = [True, False]
-        MockedSettings.return_value = mocked_settings
-
-        mock_song = MagicMock()
-        mock_song.title = 'My Song'
-        mock_song.authors_songs = []
-        mock_author = MagicMock()
-        mock_author.display_name = 'my 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)
-
-        # WHEN: I generate the Footer with default settings
-        author_list = self.media_item.generate_footer(service_item, mock_song)
-
-        # THEN: I get the following Array returned
-        assert 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'
-        assert author_list == ['my author'], 'The author list should be returned correctly with one author'
-
-    @patch(u'openlp.plugins.songs.lib.mediaitem.Settings')
-    def test_build_song_footer_one_author_hide_written_by(self, MockedSettings):
-        """
-        Test build songs footer with basic song and one author
-        """
-        # GIVEN: A Song and a Service Item, mocked settings: False for 'songs/display written by'
-        # and False for 'core/ccli number' (ccli will cause traceback if true)
-
-        mocked_settings = MagicMock()
-        mocked_settings.value.side_effect = [False, False]
-        MockedSettings.return_value = mocked_settings
-
-        mock_song = MagicMock()
-        mock_song.title = 'My Song'
-        mock_song.authors_songs = []
-        mock_author = MagicMock()
-        mock_author.display_name = 'my 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)
-
-        # WHEN: I generate the Footer with default settings
-        author_list = self.media_item.generate_footer(service_item, mock_song)
-
-        # THEN: I get the following Array returned
-        assert service_item.raw_footer == ['My Song', 'my author', 'My copyright'], \
-            'The array should be returned correctly with a song, one author and copyright, ' \
-            'text Written by should not be part of the text.'
-        assert author_list == ['my author'], 'The author list should be returned correctly with one author'
+        # GIVEN: A Song and a Service Item, mocked settings
+
+        mocked_settings = MagicMock()
+        mocked_settings.value.side_effect = [False, "", "0"]
+        MockedSettings.return_value = mocked_settings
+
+        with patch('mako.template.Template.render_unicode') as MockedRenderer:
+            mock_song = MagicMock()
+            mock_song.title = 'My Song'
+            mock_song.alternate_title = ''
+            mock_song.ccli_number = ''
+            mock_song.authors_songs = []
+            mock_author = MagicMock()
+            mock_author.display_name = 'my author'
+            mock_author_song = MagicMock()
+            mock_author_song.author = mock_author
+            mock_song.authors_songs.append(mock_author_song)
+            mock_song.copyright = 'My copyright'
+            mock_song.songbook_entries = []
+            service_item = ServiceItem(None)
+
+            # WHEN: I generate the Footer with default settings
+            author_list = self.media_item.generate_footer(service_item, mock_song)
+
+            # THEN: The mako function was called with the following arguments
+            args = {'authors_translation': [], 'authors_music_label': 'Music',
+                    'copyright': 'My copyright', 'songbook_entries': [],
+                    'alternate_title': '', 'topics': [], 'authors_music_all': [],
+                    'authors_words_label': 'Words', 'authors_music': [],
+                    'authors_words_music': [], 'ccli_number': '',
+                    'authors_none_label': 'Written by', 'title': 'My Song',
+                    'authors_words_music_label': 'Words and Music',
+                    'authors_none': ['my author'],
+                    'ccli_license_label': 'CCLI License', 'authors_words': [],
+                    'ccli_license': '0', 'authors_translation_label': 'Translation',
+                    'authors_words_all': []}
+            MockedRenderer.assert_called_once_with(**args)
+            self.assertEqual(author_list, ['my author'],
+                             'The author list should be returned correctly with one author')
 
     def test_build_song_footer_two_authors(self):
         """
@@ -382,6 +421,7 @@
         mock_author_song.author_type = AuthorType.Translation
         mock_song.authors_songs.append(mock_author_song)
         mock_song.copyright = 'My copyright'
+        mock_song.songbook_entries = []
         service_item = ServiceItem(None)
 
         # WHEN: I generate the Footer with default settings
@@ -389,7 +429,7 @@
 
         # THEN: I get the following Array returned
         assert service_item.raw_footer == ['My Song', 'Words: another author', 'Music: my author',
-                                           'Translation: translator', 'My copyright'], \
+                                           'Translation: translator', '© My copyright'], \
             'The array should be returned correctly with a song, two authors and copyright'
         assert author_list == ['another author', 'my author', 'translator'], \
             'The author list should be returned correctly with two authors'
@@ -402,6 +442,7 @@
         mock_song = MagicMock()
         mock_song.title = 'My Song'
         mock_song.copyright = 'My copyright'
+        mock_song.songbook_entries = []
         service_item = ServiceItem(None)
         Settings().setValue('core/ccli number', '1234')
 
@@ -409,7 +450,7 @@
         self.media_item.generate_footer(service_item, mock_song)
 
         # THEN: I get the following Array returned
-        assert service_item.raw_footer == ['My Song', 'My copyright', 'CCLI License: 1234'], \
+        assert 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
@@ -417,7 +458,7 @@
         self.media_item.generate_footer(service_item, mock_song)
 
         # THEN: I would get an amended footer string
-        assert service_item.raw_footer == ['My Song', 'My copyright', 'CCLI License: 4321'], \
+        assert 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'
 
     def test_build_song_footer_base_songbook(self):
@@ -431,6 +472,8 @@
         song.copyright = 'My copyright'
         song.authors_songs = []
         song.songbook_entries = []
+        song.alternate_title = ''
+        song.topics = []
         song.ccli_number = ''
         book1 = MagicMock()
         book1.name = 'My songbook'
@@ -444,15 +487,8 @@
         # WHEN: I generate the Footer with default settings
         self.media_item.generate_footer(service_item, song)
 
-        # THEN: The songbook should not be in the footer
-        assert service_item.raw_footer == ['My Song', 'My copyright']
-
-        # WHEN: I activate the "display songbook" option
-        self.media_item.display_songbook = True
-        self.media_item.generate_footer(service_item, song)
-
         # THEN: The songbook should be in the footer
-        assert service_item.raw_footer == ['My Song', 'My copyright', 'My songbook #12, Thy songbook #502A']
+        assert service_item.raw_footer == ['My Song', '© My copyright', 'My songbook #12, Thy songbook #502A']
 
     def test_build_song_footer_copyright_enabled(self):
         """
@@ -463,6 +499,7 @@
         mock_song = MagicMock()
         mock_song.title = 'My Song'
         mock_song.copyright = 'My copyright'
+        mock_song.songbook_entries = []
         service_item = ServiceItem(None)
 
         # WHEN: I generate the Footer with default settings
@@ -479,13 +516,14 @@
         mock_song = MagicMock()
         mock_song.title = 'My Song'
         mock_song.copyright = 'My copyright'
+        mock_song.songbook_entries = []
         service_item = ServiceItem(None)
 
         # WHEN: I generate the Footer with default settings
         self.media_item.generate_footer(service_item, mock_song)
 
         # THEN: The copyright symbol should not be in the footer
-        assert service_item.raw_footer == ['My Song', 'My copyright']
+        assert service_item.raw_footer == ['My Song', '© My copyright']
 
     def test_authors_match(self):
         """