Merge lp:~sam92/openlp/bug-1695620 into lp:openlp

[Samuel Mehrbrodt]

Make use of Mako for footer generation being 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)

lp:~sam92/openlp/bug-1695620 (revision 2756)
[SUCCESS] https://ci.openlp.io/job/Branch-01-Pull/2085/
[SUCCESS] https://ci.openlp.io/job/Branch-02-Functional-Tests/1995/
[SUCCESS] https://ci.openlp.io/job/Branch-03-Interface-Tests/1909/
[SUCCESS] https://ci.openlp.io/job/Branch-04a-Code_Analysis/1287/
[SUCCESS] https://ci.openlp.io/job/Branch-04b-Test_Coverage/1136/
[SUCCESS] https://ci.openlp.io/job/Branch-04c-Code_Analysis2/265/
[FAILURE] https://ci.openlp.io/job/Branch-05-AppVeyor-Tests/110/

[Tim Bentley (trb143)]

You need to run CI on this to show the success of tests.
Not sure why but this request did a full download when I did a check out. Also cannot run due to chord issues.
Cannot run this but there seem to be lots of line breaks in the footer display. As the footer area is small this will not be displayed and lead to forum comments.
You need some validation do defend against too long footers!

review: Needs Fixing

[Raoul Snyman (raoul-snyman)]

We're moving away from Python generated HTML, and using a JS system based on Reveal.js. Come chat with tgc and myself in IRC.

review: Disapprove

[Samuel Mehrbrodt (sam92)]

> Cannot run this but there seem to be lots of line breaks in the footer display. As the footer area is small this will not be displayed and lead to forum comments.

The footer has the same height as before, there should be no difference by default.

So I replaced Pystache with Mako. Should be safe to merge now I think.
As the new renderer is in a very early state atm, I can't see how I can adopt this for the new renderer. Maybe we can merge this now and adopt when the new renderer is ready?

[Tim Bentley (trb143)]

Blank lines!
Will review tomorrow as I have bible study!

review: Needs Fixing

[lp:~sam92/openlp/bug-1695620]

2757. By Samuel Mehrbrodt on 2017-06-12

Remove blank lines

2758. By Samuel Mehrbrodt on 2017-06-13

Adjust display of available placeholders to mako syntax

2759. By Samuel Mehrbrodt on 2017-06-13

Only save footer template if it has been changed

2760. By Samuel Mehrbrodt on 2019-02-08

Merge trunk

2761. By Samuel Mehrbrodt on 2019-02-08

Remove blank lines

2762. By Samuel Mehrbrodt on 2019-02-21

Merge trunk

2763. By Samuel Mehrbrodt on 2019-04-08

Merge trunk

2764. By Samuel Mehrbrodt on 2019-04-08

Fix songstab

Unmerged revisions

=== modified file 'openlp/core/lib/serviceitem.py'
--- openlp/core/lib/serviceitem.py	2017-05-17 20:06:45 +0000
+++ openlp/core/lib/serviceitem.py	2017-06-12 20:53:24 +0000
@@ -163,7 +163,8 @@
         self.items = []
         self.iconic_representation = None
         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._raw_frames = []
@@ -276,12 +277,8 @@
         else:
             log.error('Invalid value renderer: {item}'.format(item=self.service_item_type))
         self.title = clean_tags(self.title)
-        # The footer should never be None, but to be compatible with a few
-        # nightly builds between 1.9.4 and 1.9.5, we have to correct this to
-        # avoid tracebacks.
-        if self.raw_footer is None:
-            self.raw_footer = []
-        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])
 
     def add_from_image(self, path, title, background=None, thumbnail=None):
         """

=== modified file 'openlp/core/ui/maindisplay.py'
--- openlp/core/ui/maindisplay.py	2017-06-04 12:26:50 +0000
+++ openlp/core/ui/maindisplay.py	2017-06-12 20:53:24 +0000
@@ -471,8 +471,8 @@
         created_html = build_html(self.service_item, self.screen, self.is_live, background, image_bytes,
                                   plugins=self.plugin_manager.plugins)
         self.web_view.setHtml(created_html)
-        if service_item.foot_text:
-            self.footer(service_item.foot_text)
+        if service_item.footer_html:
+            self.footer(service_item.footer_html)
         # if was hidden keep it hidden
         if self.hide_mode and self.is_live and not service_item.is_media():
             if Settings().value('core/auto unblank'):

=== modified file 'openlp/core/ui/printserviceform.py'
--- openlp/core/ui/printserviceform.py	2017-06-04 12:14:23 +0000
+++ openlp/core/ui/printserviceform.py	2017-06-12 20:53:24 +0000
@@ -231,11 +231,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, classId='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	2017-06-09 06:06:49 +0000
+++ openlp/plugins/songs/lib/mediaitem.py	2017-06-12 20:53:24 +0000
@@ -23,6 +23,7 @@
 import logging
 import os
 import shutil
+import mako
 
 from PyQt5 import QtCore, QtWidgets
 from sqlalchemy.sql import and_, or_
@@ -30,7 +31,7 @@
 from openlp.core.common import Registry, AppLocation, Settings, check_directory_exists, UiStrings, translate
 from openlp.core.lib import MediaManagerItem, ItemCapabilities, PluginStatus, ServiceItemContext, \
     check_item_selected, create_separated_list
-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.common.languagemanager import get_natural_key
 from openlp.plugins.songs.forms.editsongform import EditSongForm
 from openlp.plugins.songs.forms.songmaintenanceform import SongMaintenanceForm
@@ -125,9 +126,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 retranslateUi(self):
         self.search_text_label.setText('{text}:'.format(text=UiStrings().Search))
@@ -645,12 +643,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)))
@@ -664,17 +658,47 @@
             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.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	2017-02-26 21:14:49 +0000
+++ openlp/plugins/songs/lib/songstab.py	2017-06-12 20:53:24 +0000
@@ -24,7 +24,7 @@
 
 from openlp.core.common import Settings, translate
 from openlp.core.lib import SettingsTab
-from openlp.plugins.songs.lib.ui import SongStrings
+from openlp.plugins.songs.lib.db import AuthorType
 
 
 class SongsTab(SettingsTab):
@@ -50,15 +50,6 @@
         self.add_from_service_check_box = QtWidgets.QCheckBox(self.mode_group_box)
         self.add_from_service_check_box.setObjectName('add_from_service_check_box')
         self.mode_layout.addWidget(self.add_from_service_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)
@@ -89,19 +80,35 @@
         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.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 retranslateUi(self):
         self.mode_group_box.setTitle(translate('SongsPlugin.SongsTab', 'Song related settings'))
@@ -110,12 +117,6 @@
         self.update_on_edit_check_box.setText(translate('SongsPlugin.SongsTab', 'Update service from song edit'))
         self.add_from_service_check_box.setText(translate('SongsPlugin.SongsTab',
                                                           'Import missing songs from Service files'))
-        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'))
@@ -127,6 +128,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)
@@ -140,15 +188,6 @@
     def on_add_from_service_check_box_changed(self, check_state):
         self.update_load = (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)
 
@@ -164,15 +203,15 @@
     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)
         self.tool_bar = settings.value('display songbar')
         self.update_edit = settings.value('update service on edit')
         self.update_load = settings.value('add song from service')
-        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')
@@ -180,9 +219,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)
@@ -192,6 +228,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):
@@ -200,13 +237,11 @@
         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)
+        settings.setValue('footer template', self.footer_edit_box.toPlainText())
         settings.endGroup()
         if self.tab_visited:
             self.settings_form.register_post_process('songs_config_updated')

=== modified file 'openlp/plugins/songs/songsplugin.py'
--- openlp/plugins/songs/songsplugin.py	2017-06-04 09:52:15 +0000
+++ openlp/plugins/songs/songsplugin.py	2017-06-12 20:53:24 +0000
@@ -59,9 +59,6 @@
     'songs/update service on edit': False,
     'songs/add song from service': True,
     'songs/display songbar': True,
-    'songs/display songbook': False,
-    'songs/display written by': True,
-    'songs/display copyright symbol': False,
     'songs/last directory import': '',
     'songs/last directory export': '',
     'songs/songselect username': '',
@@ -71,6 +68,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	2017-06-05 21:41:29 +0000
+++ tests/functional/openlp_plugins/songs/test_mediaitem.py	2017-06-12 20:53:24 +0000
@@ -34,6 +34,62 @@
 
 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):
@@ -301,65 +358,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
-        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')
-
-    @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
-        self.assertEqual(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.')
-        self.assertEqual(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):
         """
@@ -388,6 +425,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
@@ -395,7 +433,7 @@
 
         # THEN: I get the following Array returned
         self.assertEqual(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')
         self.assertEqual(author_list, ['another author', 'my author', 'translator'],
                          'The author list should be returned correctly with two authors')
@@ -408,6 +446,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')
 
@@ -415,7 +454,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 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
@@ -423,7 +462,7 @@
         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 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')
 
     def test_build_song_footer_base_songbook(self):
@@ -436,6 +475,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"
@@ -449,15 +490,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
-        self.assertEqual(service_item.raw_footer, ['My Song', 'My copyright'])
-
-        # WHEN: I activate the "display songbook" option
-        self.media_item.display_songbook = True
-        self.media_item.generate_footer(service_item, song)
-
         # THEN: The songbook should be in the footer
-        self.assertEqual(service_item.raw_footer, ['My Song', 'My copyright', 'My songbook #12, Thy songbook #502A'])
+        self.assertEqual(service_item.raw_footer, ['My Song', '© My copyright', 'My songbook #12, Thy songbook #502A'])
 
     def test_build_song_footer_copyright_enabled(self):
         """
@@ -468,6 +502,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
@@ -484,13 +519,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
-        self.assertEqual(service_item.raw_footer, ['My Song', 'My copyright'])
+        self.assertEqual(service_item.raw_footer, ['My Song', '© My copyright'])
 
     def test_authors_match(self):
         """