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

Proposed by Samuel Mehrbrodt
Status: Superseded
Proposed branch: lp:~sam92/openlp/bug-1695620
Merge into: lp:openlp
Diff against target: 724 lines (+284/-155)
6 files modified
openlp/core/lib/serviceitem.py (+5/-3)
openlp/core/ui/printserviceform.py (+5/-5)
openlp/plugins/songs/lib/mediaitem.py (+40/-36)
openlp/plugins/songs/lib/songstab.py (+72/-37)
openlp/plugins/songs/songsplugin.py (+55/-5)
tests/functional/openlp_plugins/songs/test_mediaitem.py (+107/-69)
To merge this branch: bzr merge lp:~sam92/openlp/bug-1695620
Reviewer Review Type Date Requested Status
Raoul Snyman Needs Fixing
Tim Bentley Approve
Tomas Groth Approve
Review via email: mp+363518@code.launchpad.net

This proposal supersedes a proposal from 2019-02-08.

This proposal has been superseded by a proposal from 2019-04-08.

Commit message

Make footer configurable

Description of the change

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)

To post a comment you must log in.
Revision history for this message
Tim Bentley (trb143) wrote : Posted in a previous version of this proposal

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
Revision history for this message
Raoul Snyman (raoul-snyman) wrote : Posted in a previous version of this proposal

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
Revision history for this message
Samuel Mehrbrodt (sam92) wrote : Posted in a previous version of this proposal

> 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?

Revision history for this message
Tim Bentley (trb143) wrote : Posted in a previous version of this proposal

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

review: Needs Fixing
Revision history for this message
Samuel Mehrbrodt (sam92) wrote : Posted in a previous version of this proposal

So is there a chance this can be reviewed now?

> Blank lines!
What do you mean with that?

Revision history for this message
Raoul Snyman (raoul-snyman) wrote : Posted in a previous version of this proposal

Linux tests passed!

Revision history for this message
Raoul Snyman (raoul-snyman) wrote : Posted in a previous version of this proposal

Linting passed!

Revision history for this message
Raoul Snyman (raoul-snyman) wrote : Posted in a previous version of this proposal

macOS tests passed!

Revision history for this message
Raoul Snyman (raoul-snyman) wrote :

Linux tests passed!

Revision history for this message
Raoul Snyman (raoul-snyman) wrote :

Linting passed!

Revision history for this message
Raoul Snyman (raoul-snyman) wrote :

macOS tests passed!

Revision history for this message
Tomas Groth (tomasgroth) wrote :

Looks good to me!

review: Approve
Revision history for this message
Tim Bentley (trb143) wrote :

Template should be in its own file so can be tested and not duplicated.
Not happy with the if conditions being removed these have been added as we sometimes get broken databases in the wild and this needs to be guarded against.

review: Needs Fixing
Revision history for this message
Samuel Mehrbrodt (sam92) wrote :

@Tim: As discussed on IRC, could you please review again?
I had to remove the conditions so that the variables are always available to the template, even if they are empty. They were there to make sure that no empty lines were added to the footer. But now this checking is done in the template.

Also wrt the template in its own file, after discussion we came to agreement that it's not worth the hassle (packaging etc). So we keep it just as a config string.

Revision history for this message
Tim Bentley (trb143) :
review: Approve
Revision history for this message
Raoul Snyman (raoul-snyman) wrote :

So, I like the principle, but I really don't think that using Mako is necessary. Do you think it's plausible to just use string formatting or string templating?

review: Needs Information
Revision history for this message
Samuel Mehrbrodt (sam92) wrote :

> Do you think it's plausible to just use string formatting or string templating?

Hm while that would allow adding a bit of markup around the strings, you can't add any control structures.

With mako you can do much more, e.g. concatenate multiple authors into one line (as we do using join), display them each on it own line.

The use case I'm especially interested in: Pick just one songbook to be displayed. Using Mako I can just write Python code to only show the songbook we are actually using in the service.

I'm sure there are more valid use cases for this.

Revision history for this message
Raoul Snyman (raoul-snyman) wrote :

You can do the same things in Python. The thing is, we no longer use Mako anywhere else. It's such a small usage, I'd really like it if we could remove the dependency.

I know we're trying to make a customisable footer, but is there any way we can do it without Mako and without introducing another dependency?

Look, if there's no other way around it, then I'll approve this, I'm just trying to reduce/minimize dependencies where possible.

Revision history for this message
Raoul Snyman (raoul-snyman) wrote :

I'm getting conflicts when merging with trunk, can you just re-merge trunk and fix the conflicts please?

review: Needs Fixing
lp:~sam92/openlp/bug-1695620 updated
2763. By Samuel Mehrbrodt

Merge trunk

2764. By Samuel Mehrbrodt

Fix songstab

Unmerged revisions

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'openlp/core/lib/serviceitem.py'
2--- openlp/core/lib/serviceitem.py 2019-03-17 10:36:12 +0000
3+++ openlp/core/lib/serviceitem.py 2019-04-08 21:06:37 +0000
4@@ -81,7 +81,8 @@
5 self.items = []
6 self.icon = UiIcons().default
7 self.raw_footer = []
8- self.foot_text = ''
9+ # Plugins can set footer_html themselves. If they don't, it will be generated from raw_footer.
10+ self.footer_html = ''
11 self.theme = None
12 self.service_item_type = None
13 self.unique_identifier = 0
14@@ -165,7 +166,8 @@
15 # the dict instead of rendering them again.
16 previous_pages = {}
17 index = 0
18- self.foot_text = '<br>'.join([_f for _f in self.raw_footer if _f])
19+ if not self.footer_html:
20+ self.footer_html = '<br>'.join([_f for _f in self.raw_footer if _f])
21 for raw_slide in self.slides:
22 verse_tag = raw_slide['verse']
23 if verse_tag in previous_pages and previous_pages[verse_tag][0] == raw_slide:
24@@ -178,7 +180,7 @@
25 'title': raw_slide['title'],
26 'text': render_tags(page),
27 'verse': index,
28- 'footer': self.foot_text,
29+ 'footer': self.footer_html,
30 }
31 self._rendered_slides.append(rendered_slide)
32 display_slide = {
33
34=== modified file 'openlp/core/ui/printserviceform.py'
35--- openlp/core/ui/printserviceform.py 2019-02-14 15:09:09 +0000
36+++ openlp/core/ui/printserviceform.py 2019-04-08 21:06:37 +0000
37@@ -235,11 +235,11 @@
38 for slide in range(len(item.get_frames())):
39 self._add_element('li', item.get_frame_title(slide), ol)
40 # add footer
41- foot_text = item.foot_text
42- foot_text = foot_text.partition('<br>')[2]
43- if foot_text:
44- foot_text = html.escape(foot_text.replace('<br>', '\n'))
45- self._add_element('div', foot_text.replace('\n', '<br>'), parent=div, class_id='itemFooter')
46+ footer_html = item.footer_html
47+ footer_html = footer_html.partition('<br>')[2]
48+ if footer_html:
49+ footer_html = html.escape(footer_html.replace('<br>', '\n'))
50+ self._add_element('div', footer_html.replace('\n', '<br>'), parent=div, classId='itemFooter')
51 # Add service items' notes.
52 if self.notes_check_box.isChecked():
53 if item.notes:
54
55=== modified file 'openlp/plugins/songs/lib/mediaitem.py'
56--- openlp/plugins/songs/lib/mediaitem.py 2019-03-28 20:19:15 +0000
57+++ openlp/plugins/songs/lib/mediaitem.py 2019-04-08 21:06:37 +0000
58@@ -21,6 +21,7 @@
59 ###############################################################################
60 import logging
61 import os
62+import mako
63
64 from PyQt5 import QtCore, QtWidgets
65 from sqlalchemy.sql import and_, or_
66@@ -35,7 +36,7 @@
67 from openlp.core.lib.mediamanageritem import MediaManagerItem
68 from openlp.core.lib.plugin import PluginStatus
69 from openlp.core.lib.serviceitem import ItemCapabilities
70-from openlp.core.lib.ui import create_widget_action
71+from openlp.core.lib.ui import create_widget_action, critical_error_message_box
72 from openlp.core.ui.icons import UiIcons
73 from openlp.plugins.songs.forms.editsongform import EditSongForm
74 from openlp.plugins.songs.forms.songexportform import SongExportForm
75@@ -131,9 +132,6 @@
76 self.is_search_as_you_type_enabled = Settings().value('advanced/search as type')
77 self.update_service_on_edit = Settings().value(self.settings_section + '/update service on edit')
78 self.add_song_from_service = Settings().value(self.settings_section + '/add song from service')
79- self.display_songbook = Settings().value(self.settings_section + '/display songbook')
80- self.display_written_by_text = Settings().value(self.settings_section + '/display written by')
81- self.display_copyright_symbol = Settings().value(self.settings_section + '/display copyright symbol')
82
83 def retranslate_ui(self):
84 self.search_text_label.setText('{text}:'.format(text=UiStrings().Search))
85@@ -677,12 +675,8 @@
86 item.raw_footer = []
87 item.raw_footer.append(song.title)
88 if authors_none:
89- # If the setting for showing "Written by:" is enabled, show it before unspecified authors.
90- if Settings().value('songs/display written by'):
91- item.raw_footer.append("{text}: {authors}".format(text=translate('OpenLP.Ui', 'Written by'),
92- authors=create_separated_list(authors_none)))
93- else:
94- item.raw_footer.append("{authors}".format(authors=create_separated_list(authors_none)))
95+ item.raw_footer.append("{text}: {authors}".format(text=translate('OpenLP.Ui', 'Written by'),
96+ authors=create_separated_list(authors_none)))
97 if authors_words_music:
98 item.raw_footer.append("{text}: {authors}".format(text=AuthorType.Types[AuthorType.WordsAndMusic],
99 authors=create_separated_list(authors_words_music)))
100@@ -696,34 +690,44 @@
101 item.raw_footer.append("{text}: {authors}".format(text=AuthorType.Types[AuthorType.Translation],
102 authors=create_separated_list(authors_translation)))
103 if song.copyright:
104- if self.display_copyright_symbol:
105- item.raw_footer.append("{symbol} {song}".format(symbol=SongStrings.CopyrightSymbol,
106- song=song.copyright))
107- else:
108- item.raw_footer.append(song.copyright)
109- if self.display_songbook and song.songbook_entries:
110- songbooks = [str(songbook_entry) for songbook_entry in song.songbook_entries]
111+ item.raw_footer.append("{symbol} {song}".format(symbol=SongStrings.CopyrightSymbol,
112+ song=song.copyright))
113+ songbooks = [str(songbook_entry) for songbook_entry in song.songbook_entries]
114+ if song.songbook_entries:
115 item.raw_footer.append(", ".join(songbooks))
116 if Settings().value('core/ccli number'):
117- item.raw_footer.append(translate('SongsPlugin.MediaItem',
118- 'CCLI License: ') + Settings().value('core/ccli number'))
119- item.metadata.append('<em>{label}:</em> {title}'.format(label=translate('SongsPlugin.MediaItem', 'Title'),
120- title=song.title))
121- if song.alternate_title:
122- item.metadata.append('<em>{label}:</em> {title}'.
123- format(label=translate('SongsPlugin.MediaItem', 'Alt Title'),
124- title=song.alternate_title))
125- if song.songbook_entries:
126- for songbook_entry in song.songbook_entries:
127- item.metadata.append('<em>{label}:</em> {book}/{num}/{pub}'.
128- format(label=translate('SongsPlugin.MediaItem', 'Songbook'),
129- book=songbook_entry.songbook.name,
130- num=songbook_entry.entry,
131- pub=songbook_entry.songbook.publisher))
132- if song.topics:
133- for topics in song.topics:
134- item.metadata.append('<em>{label}:</em> {topic}'.
135- format(label=translate('SongsPlugin.MediaItem', 'Topic'), topic=topics.name))
136+ item.raw_footer.append(translate('SongsPlugin.MediaItem', 'CCLI License: ') +
137+ Settings().value('core/ccli number'))
138+ footer_template = Settings().value('songs/footer template')
139+ # Keep this in sync with the list in songstab.py
140+ vars = {
141+ 'title': song.title,
142+ 'alternate_title': song.alternate_title,
143+ 'authors_none_label': translate('OpenLP.Ui', 'Written by'),
144+ 'authors_none': authors_none,
145+ 'authors_words_label': AuthorType.Types[AuthorType.Words],
146+ 'authors_words': authors_words,
147+ 'authors_music_label': AuthorType.Types[AuthorType.Music],
148+ 'authors_music': authors_music,
149+ 'authors_words_music_label': AuthorType.Types[AuthorType.WordsAndMusic],
150+ 'authors_words_music': authors_words_music,
151+ 'authors_translation_label': AuthorType.Types[AuthorType.Translation],
152+ 'authors_translation': authors_translation,
153+ 'authors_words_all': authors_words + authors_words_music,
154+ 'authors_music_all': authors_music + authors_words_music,
155+ 'copyright': song.copyright,
156+ 'songbook_entries': songbooks,
157+ 'ccli_license': Settings().value('core/ccli number'),
158+ 'ccli_license_label': translate('SongsPlugin.MediaItem', 'CCLI License'),
159+ 'ccli_number': song.ccli_number,
160+ 'topics': [topic.name for topic in song.topics]
161+ }
162+ try:
163+ item.footer_html = mako.template.Template(footer_template).render_unicode(**vars).replace('\n', '')
164+ except mako.exceptions.SyntaxException:
165+ log.error('Failed to render Song footer html:\n' + mako.exceptions.text_error_template().render())
166+ critical_error_message_box(message=translate('SongsPlugin.MediaItem',
167+ 'Failed to render Song footer html.\nSee log for details'))
168 return authors_all
169
170 def service_load(self, item):
171
172=== modified file 'openlp/plugins/songs/lib/songstab.py'
173--- openlp/plugins/songs/lib/songstab.py 2019-02-22 07:34:40 +0000
174+++ openlp/plugins/songs/lib/songstab.py 2019-04-08 21:06:37 +0000
175@@ -25,7 +25,7 @@
176 from openlp.core.common.i18n import translate
177 from openlp.core.common.settings import Settings
178 from openlp.core.lib.settingstab import SettingsTab
179-from openlp.plugins.songs.lib.ui import SongStrings
180+from openlp.plugins.songs.lib.db import AuthorType
181
182
183 class SongsTab(SettingsTab):
184@@ -54,15 +54,6 @@
185 self.songbook_slide_check_box = QtWidgets.QCheckBox(self.mode_group_box)
186 self.songbook_slide_check_box.setObjectName('songbook_slide_check_box')
187 self.mode_layout.addWidget(self.songbook_slide_check_box)
188- self.display_songbook_check_box = QtWidgets.QCheckBox(self.mode_group_box)
189- self.display_songbook_check_box.setObjectName('songbook_check_box')
190- self.mode_layout.addWidget(self.display_songbook_check_box)
191- self.display_written_by_check_box = QtWidgets.QCheckBox(self.mode_group_box)
192- self.display_written_by_check_box.setObjectName('written_by_check_box')
193- self.mode_layout.addWidget(self.display_written_by_check_box)
194- self.display_copyright_check_box = QtWidgets.QCheckBox(self.mode_group_box)
195- self.display_copyright_check_box.setObjectName('copyright_check_box')
196- self.mode_layout.addWidget(self.display_copyright_check_box)
197 self.left_layout.addWidget(self.mode_group_box)
198 # Chords group box
199 self.chords_group_box = QtWidgets.QGroupBox(self.left_column)
200@@ -93,20 +84,34 @@
201 self.neolatin_notation_radio_button.setObjectName('neolatin_notation_radio_button')
202 self.chords_layout.addWidget(self.neolatin_notation_radio_button)
203 self.left_layout.addWidget(self.chords_group_box)
204+ # Footer group box
205+ self.footer_group_box = QtWidgets.QGroupBox(self.left_column)
206+ self.footer_group_box.setObjectName('footer_group_box')
207+ self.footer_layout = QtWidgets.QVBoxLayout(self.footer_group_box)
208+ self.footer_layout.setObjectName('chords_layout')
209+ self.footer_info_label = QtWidgets.QLabel(self.footer_group_box)
210+ self.footer_layout.addWidget(self.footer_info_label)
211+ self.footer_placeholder_info = QtWidgets.QTextEdit(self.footer_group_box)
212+ self.footer_layout.addWidget(self.footer_placeholder_info)
213+ self.footer_desc_label = QtWidgets.QLabel(self.footer_group_box)
214+ self.footer_layout.addWidget(self.footer_desc_label)
215+ self.footer_edit_box = QtWidgets.QTextEdit(self.footer_group_box)
216+ self.footer_layout.addWidget(self.footer_edit_box)
217+ self.footer_reset_button = QtWidgets.QPushButton(self.footer_group_box)
218+ self.footer_layout.addWidget(self.footer_reset_button, alignment=QtCore.Qt.AlignRight)
219+ self.right_layout.addWidget(self.footer_group_box)
220 self.left_layout.addStretch()
221 self.right_layout.addStretch()
222 self.tool_bar_active_check_box.stateChanged.connect(self.on_tool_bar_active_check_box_changed)
223 self.update_on_edit_check_box.stateChanged.connect(self.on_update_on_edit_check_box_changed)
224 self.add_from_service_check_box.stateChanged.connect(self.on_add_from_service_check_box_changed)
225 self.songbook_slide_check_box.stateChanged.connect(self.on_songbook_slide_check_box_changed)
226- self.display_songbook_check_box.stateChanged.connect(self.on_songbook_check_box_changed)
227- self.display_written_by_check_box.stateChanged.connect(self.on_written_by_check_box_changed)
228- self.display_copyright_check_box.stateChanged.connect(self.on_copyright_check_box_changed)
229 self.mainview_chords_check_box.stateChanged.connect(self.on_mainview_chords_check_box_changed)
230 self.disable_chords_import_check_box.stateChanged.connect(self.on_disable_chords_import_check_box_changed)
231 self.english_notation_radio_button.clicked.connect(self.on_english_notation_button_clicked)
232 self.german_notation_radio_button.clicked.connect(self.on_german_notation_button_clicked)
233 self.neolatin_notation_radio_button.clicked.connect(self.on_neolatin_notation_button_clicked)
234+ self.footer_reset_button.clicked.connect(self.on_footer_reset_button_clicked)
235
236 def retranslate_ui(self):
237 self.mode_group_box.setTitle(translate('SongsPlugin.SongsTab', 'Song related settings'))
238@@ -117,12 +122,6 @@
239 'Import missing songs from Service files'))
240 self.songbook_slide_check_box.setText(translate('SongsPlugin.SongsTab',
241 'Add Songbooks as first slide'))
242- self.display_songbook_check_box.setText(translate('SongsPlugin.SongsTab', 'Display songbook in footer'))
243- self.display_written_by_check_box.setText(translate(
244- 'SongsPlugin.SongsTab', 'Show "Written by:" in footer for unspecified authors'))
245- self.display_copyright_check_box.setText(translate('SongsPlugin.SongsTab',
246- 'Display "{symbol}" symbol before copyright '
247- 'info').format(symbol=SongStrings.CopyrightSymbol))
248 self.chords_info_label.setText(translate('SongsPlugin.SongsTab', 'If enabled all text between "[" and "]" will '
249 'be regarded as chords.'))
250 self.chords_group_box.setTitle(translate('SongsPlugin.SongsTab', 'Chords'))
251@@ -134,6 +133,53 @@
252 self.german_notation_radio_button.setText(translate('SongsPlugin.SongsTab', 'German') + ' (C-D-E-F-G-A-H)')
253 self.neolatin_notation_radio_button.setText(
254 translate('SongsPlugin.SongsTab', 'Neo-Latin') + ' (Do-Re-Mi-Fa-Sol-La-Si)')
255+ self.footer_group_box.setTitle(translate('SongsPlugin.SongsTab', 'Footer'))
256+ # Keep this in sync with the list in mediaitem.py
257+ const = '<code>"{}"</code>'
258+ placeholders = [
259+ # placeholder, description, can be empty, is a list
260+ ['title', translate('SongsPlugin.SongsTab', 'Song Title'), False, False],
261+ ['alternate_title', translate('SongsPlugin.SongsTab', 'Alternate Title'), True, False],
262+ ['written_by', const.format(translate('SongsPlugin.SongsTab', 'Written By')), True, False],
263+ ['authors_none', translate('SongsPlugin.SongsTab', 'Authors when type is not set'), False, True],
264+ ['authors_words_label', const.format(AuthorType.Types[AuthorType.Words]), False, False],
265+ ['authors_words', translate('SongsPlugin.SongsTab', 'Authors (Type "Words")'), False, True],
266+ ['authors_music_label', const.format(AuthorType.Types[AuthorType.Music]), False, False],
267+ ['authors_music', translate('SongsPlugin.SongsTab', 'Authors (Type "Music")'), False, True],
268+ ['authors_words_music_label', const.format(AuthorType.Types[AuthorType.WordsAndMusic]), False, False],
269+ ['authors_words_music', translate('SongsPlugin.SongsTab', 'Authors (Type "Words and Music")'), False, True],
270+ ['authors_translation_label', const.format(AuthorType.Types[AuthorType.Translation]), False, False],
271+ ['authors_translation', translate('SongsPlugin.SongsTab', 'Authors (Type "Translation")'), False, True],
272+ ['authors_words_all', translate('SongsPlugin.SongsTab', 'Authors (Type "Words" & "Words and Music")'),
273+ False, True],
274+ ['authors_music_all', translate('SongsPlugin.SongsTab', 'Authors (Type "Music" & "Words and Music")'),
275+ False, True],
276+ ['copyright', translate('SongsPlugin.SongsTab', 'Copyright information'), True, False],
277+ ['songbook_entries', translate('SongsPlugin.SongsTab', 'Songbook Entries'), False, True],
278+ ['ccli_license', translate('SongsPlugin.SongsTab', 'CCLI License'), True, False],
279+ ['ccli_license_label', const.format(translate('SongsPlugin.SongsTab', 'CCLI License')), False, False],
280+ ['ccli_number', translate('SongsPlugin.SongsTab', 'Song CCLI Number'), True, False],
281+ ['topics', translate('SongsPlugin.SongsTab', 'Topics'), False, True],
282+ ]
283+ placeholder_info = '<table style="background: #eee">\n<tr><th><b>{ph}</b></th><th><b>{desc}</b></th></tr>\n'\
284+ .format(ph=translate('SongsPlugin.SongsTab', 'Placeholder'),
285+ desc=translate('SongsPlugin.SongsTab', 'Description'))
286+ for placeholder in placeholders:
287+ placeholder_info += '<tr><td>${{{pl}}}</td><td>{des}{opt}</td></tr>\n'\
288+ .format(pl=placeholder[0], des=placeholder[1],
289+ opt=('&nbsp;¹' if placeholder[2] else '') +
290+ ('&nbsp;²' if placeholder[3] else ''))
291+ placeholder_info += '</table>'
292+ placeholder_info += '\n<br/>¹ {}'.format(translate('SongsPlugin.SongsTab', 'can be empty'))
293+ placeholder_info += '\n<br/>² {}'.format(translate('SongsPlugin.SongsTab', 'list of entries, can be empty'))
294+ self.footer_placeholder_info.setHtml(placeholder_info)
295+ self.footer_placeholder_info.setReadOnly(True)
296+
297+ self.footer_info_label.setText(translate('SongsPlugin.SongsTab', 'How to use Footers:'))
298+ self.footer_desc_label.setText('{} (<a href="http://docs.makotemplates.org">{}</a>):'
299+ .format(translate('SongsPlugin.SongsTab', 'Footer Template'),
300+ translate('SongsPlugin.SongsTab', 'Mako Syntax')))
301+ self.footer_reset_button.setText(translate('SongsPlugin.SongsTab', 'Reset Template'))
302
303 def on_search_as_type_check_box_changed(self, check_state):
304 self.song_search = (check_state == QtCore.Qt.Checked)
305@@ -150,15 +196,6 @@
306 def on_songbook_slide_check_box_changed(self, check_state):
307 self.songbook_slide = (check_state == QtCore.Qt.Checked)
308
309- def on_songbook_check_box_changed(self, check_state):
310- self.display_songbook = (check_state == QtCore.Qt.Checked)
311-
312- def on_written_by_check_box_changed(self, check_state):
313- self.display_written_by = (check_state == QtCore.Qt.Checked)
314-
315- def on_copyright_check_box_changed(self, check_state):
316- self.display_copyright_symbol = (check_state == QtCore.Qt.Checked)
317-
318 def on_mainview_chords_check_box_changed(self, check_state):
319 self.mainview_chords = (check_state == QtCore.Qt.Checked)
320
321@@ -174,6 +211,9 @@
322 def on_neolatin_notation_button_clicked(self):
323 self.chord_notation = 'neo-latin'
324
325+ def on_footer_reset_button_clicked(self):
326+ self.footer_edit_box.setPlainText(Settings().get_default_value('songs/footer template'))
327+
328 def load(self):
329 settings = Settings()
330 settings.beginGroup(self.settings_section)
331@@ -181,9 +221,6 @@
332 self.update_edit = settings.value('update service on edit')
333 self.update_load = settings.value('add song from service')
334 self.songbook_slide = settings.value('add songbook slide')
335- self.display_songbook = settings.value('display songbook')
336- self.display_written_by = settings.value('display written by')
337- self.display_copyright_symbol = settings.value('display copyright symbol')
338 self.enable_chords = settings.value('enable chords')
339 self.chord_notation = settings.value('chord notation')
340 self.mainview_chords = settings.value('mainview chords')
341@@ -191,9 +228,6 @@
342 self.tool_bar_active_check_box.setChecked(self.tool_bar)
343 self.update_on_edit_check_box.setChecked(self.update_edit)
344 self.add_from_service_check_box.setChecked(self.update_load)
345- self.display_songbook_check_box.setChecked(self.display_songbook)
346- self.display_written_by_check_box.setChecked(self.display_written_by)
347- self.display_copyright_check_box.setChecked(self.display_copyright_symbol)
348 self.chords_group_box.setChecked(self.enable_chords)
349 self.mainview_chords_check_box.setChecked(self.mainview_chords)
350 self.disable_chords_import_check_box.setChecked(self.disable_chords_import)
351@@ -203,6 +237,7 @@
352 self.neolatin_notation_radio_button.setChecked(True)
353 else:
354 self.english_notation_radio_button.setChecked(True)
355+ self.footer_edit_box.setPlainText(settings.value('footer template'))
356 settings.endGroup()
357
358 def save(self):
359@@ -211,13 +246,13 @@
360 settings.setValue('display songbar', self.tool_bar)
361 settings.setValue('update service on edit', self.update_edit)
362 settings.setValue('add song from service', self.update_load)
363- settings.setValue('display songbook', self.display_songbook)
364- settings.setValue('display written by', self.display_written_by)
365- settings.setValue('display copyright symbol', self.display_copyright_symbol)
366 settings.setValue('enable chords', self.chords_group_box.isChecked())
367 settings.setValue('mainview chords', self.mainview_chords)
368 settings.setValue('disable chords import', self.disable_chords_import)
369 settings.setValue('chord notation', self.chord_notation)
370+ # Only save footer template if it has been changed. This allows future updates
371+ if self.footer_edit_box.toPlainText() != Settings().get_default_value('songs/footer template'):
372+ settings.setValue('footer template', self.footer_edit_box.toPlainText())
373 settings.setValue('add songbook slide', self.songbook_slide)
374 settings.endGroup()
375 if self.tab_visited:
376
377=== modified file 'openlp/plugins/songs/songsplugin.py'
378--- openlp/plugins/songs/songsplugin.py 2019-03-16 10:58:59 +0000
379+++ openlp/plugins/songs/songsplugin.py 2019-04-08 21:06:37 +0000
380@@ -66,11 +66,8 @@
381 'songs/add song from service': True,
382 'songs/add songbook slide': False,
383 'songs/display songbar': True,
384- 'songs/display songbook': False,
385- 'songs/display written by': True,
386- 'songs/display copyright symbol': False,
387- 'songs/last directory import': None,
388- 'songs/last directory export': None,
389+ 'songs/last directory import': '',
390+ 'songs/last directory export': '',
391 'songs/songselect username': '',
392 'songs/songselect password': '',
393 'songs/songselect searches': '',
394@@ -78,6 +75,59 @@
395 'songs/chord notation': 'english', # Can be english, german or neo-latin
396 'songs/mainview chords': False,
397 'songs/disable chords import': False,
398+ 'songs/footer template': """\
399+${title}<br/>
400+
401+%if authors_none:
402+ <%
403+ authors = ", ".join(authors_none)
404+ %>
405+ ${authors_none_label}:&nbsp;${authors}<br/>
406+%endif
407+
408+%if authors_words_music:
409+ <%
410+ authors = ", ".join(authors_words_music)
411+ %>
412+ ${authors_words_music_label}:&nbsp;${authors}<br/>
413+%endif
414+
415+%if authors_words:
416+ <%
417+ authors = ", ".join(authors_words)
418+ %>
419+ ${authors_words_label}:&nbsp;${authors}<br/>
420+%endif
421+
422+%if authors_music:
423+ <%
424+ authors = ", ".join(authors_music)
425+ %>
426+ ${authors_music_label}:&nbsp;${authors}<br/>
427+%endif
428+
429+%if authors_translation:
430+ <%
431+ authors = ", ".join(authors_translation)
432+ %>
433+ ${authors_translation_label}:&nbsp;${authors}<br/>
434+%endif
435+
436+%if copyright:
437+ &copy;&nbsp;${copyright}<br/>
438+%endif
439+
440+%if songbook_entries:
441+ <%
442+ entries = ", ".join(songbook_entries)
443+ %>
444+ ${entries}<br/>
445+%endif
446+
447+%if ccli_license:
448+ ${ccli_license_label}&nbsp;${ccli_license}<br/>
449+%endif
450+""",
451 }
452
453
454
455=== modified file 'tests/functional/openlp_plugins/songs/test_mediaitem.py'
456--- tests/functional/openlp_plugins/songs/test_mediaitem.py 2019-02-14 15:09:09 +0000
457+++ tests/functional/openlp_plugins/songs/test_mediaitem.py 2019-04-08 21:06:37 +0000
458@@ -34,6 +34,62 @@
459 from openlp.plugins.songs.lib.mediaitem import SongMediaItem
460 from tests.helpers.testmixin import TestMixin
461
462+__default_settings__ = {
463+ 'songs/footer template': """
464+${title}<br/>
465+
466+%if authors_none:
467+ <%
468+ authors = ", ".join(authors_none)
469+ %>
470+ ${authors_none_label}:&nbsp;${authors}<br/>
471+%endif
472+
473+%if authors_words_music:
474+ <%
475+ authors = ", ".join(authors_words_music)
476+ %>
477+ ${authors_words_music_label}:&nbsp;${authors}<br/>
478+%endif
479+
480+%if authors_words:
481+ <%
482+ authors = ", ".join(authors_words)
483+ %>
484+ ${authors_words_label}:&nbsp;${authors}<br/>
485+%endif
486+
487+%if authors_music:
488+ <%
489+ authors = ", ".join(authors_music)
490+ %>
491+ ${authors_music_label}:&nbsp;${authors}<br/>
492+%endif
493+
494+%if authors_translation:
495+ <%
496+ authors = ", ".join(authors_translation)
497+ %>
498+ ${authors_translation_label}:&nbsp;${authors}<br/>
499+%endif
500+
501+%if copyright:
502+ &copy;&nbsp;${copyright}<br/>
503+%endif
504+
505+%if songbook_entries:
506+ <%
507+ entries = ", ".join(songbook_entries)
508+ %>
509+ ${entries}<br/>
510+%endif
511+
512+%if ccli_license:
513+ ${ccli_license_label}&nbsp;${ccli_license}<br/>
514+%endif
515+"""
516+}
517+
518
519 class TestMediaItem(TestCase, TestMixin):
520 """
521@@ -61,6 +117,7 @@
522 self.media_item.display_copyright_symbol = False
523 self.setup_application()
524 self.build_settings()
525+ Settings().extend_default_settings(__default_settings__)
526 QtCore.QLocale.setDefault(QtCore.QLocale('en_GB'))
527
528 def tearDown(self):
529@@ -297,63 +354,45 @@
530 """
531 Test build songs footer with basic song and one author
532 """
533- # GIVEN: A Song and a Service Item, mocked settings: True for 'songs/display written by'
534- # and False for 'core/ccli number' (ccli will cause traceback if true)
535-
536- mocked_settings = MagicMock()
537- mocked_settings.value.side_effect = [True, False]
538- MockedSettings.return_value = mocked_settings
539-
540- mock_song = MagicMock()
541- mock_song.title = 'My Song'
542- mock_song.authors_songs = []
543- mock_author = MagicMock()
544- mock_author.display_name = 'my author'
545- mock_author_song = MagicMock()
546- mock_author_song.author = mock_author
547- mock_song.authors_songs.append(mock_author_song)
548- mock_song.copyright = 'My copyright'
549- service_item = ServiceItem(None)
550-
551- # WHEN: I generate the Footer with default settings
552- author_list = self.media_item.generate_footer(service_item, mock_song)
553-
554- # THEN: I get the following Array returned
555- assert service_item.raw_footer == ['My Song', 'Written by: my author', 'My copyright'], \
556- 'The array should be returned correctly with a song, one author and copyright'
557- assert author_list == ['my author'], 'The author list should be returned correctly with one author'
558-
559- @patch(u'openlp.plugins.songs.lib.mediaitem.Settings')
560- def test_build_song_footer_one_author_hide_written_by(self, MockedSettings):
561- """
562- Test build songs footer with basic song and one author
563- """
564- # GIVEN: A Song and a Service Item, mocked settings: False for 'songs/display written by'
565- # and False for 'core/ccli number' (ccli will cause traceback if true)
566-
567- mocked_settings = MagicMock()
568- mocked_settings.value.side_effect = [False, False]
569- MockedSettings.return_value = mocked_settings
570-
571- mock_song = MagicMock()
572- mock_song.title = 'My Song'
573- mock_song.authors_songs = []
574- mock_author = MagicMock()
575- mock_author.display_name = 'my author'
576- mock_author_song = MagicMock()
577- mock_author_song.author = mock_author
578- mock_song.authors_songs.append(mock_author_song)
579- mock_song.copyright = 'My copyright'
580- service_item = ServiceItem(None)
581-
582- # WHEN: I generate the Footer with default settings
583- author_list = self.media_item.generate_footer(service_item, mock_song)
584-
585- # THEN: I get the following Array returned
586- assert service_item.raw_footer == ['My Song', 'my author', 'My copyright'], \
587- 'The array should be returned correctly with a song, one author and copyright, ' \
588- 'text Written by should not be part of the text.'
589- assert author_list == ['my author'], 'The author list should be returned correctly with one author'
590+ # GIVEN: A Song and a Service Item, mocked settings
591+
592+ mocked_settings = MagicMock()
593+ mocked_settings.value.side_effect = [False, "", "0"]
594+ MockedSettings.return_value = mocked_settings
595+
596+ with patch('mako.template.Template.render_unicode') as MockedRenderer:
597+ mock_song = MagicMock()
598+ mock_song.title = 'My Song'
599+ mock_song.alternate_title = ''
600+ mock_song.ccli_number = ''
601+ mock_song.authors_songs = []
602+ mock_author = MagicMock()
603+ mock_author.display_name = 'my author'
604+ mock_author_song = MagicMock()
605+ mock_author_song.author = mock_author
606+ mock_song.authors_songs.append(mock_author_song)
607+ mock_song.copyright = 'My copyright'
608+ mock_song.songbook_entries = []
609+ service_item = ServiceItem(None)
610+
611+ # WHEN: I generate the Footer with default settings
612+ author_list = self.media_item.generate_footer(service_item, mock_song)
613+
614+ # THEN: The mako function was called with the following arguments
615+ args = {'authors_translation': [], 'authors_music_label': 'Music',
616+ 'copyright': 'My copyright', 'songbook_entries': [],
617+ 'alternate_title': '', 'topics': [], 'authors_music_all': [],
618+ 'authors_words_label': 'Words', 'authors_music': [],
619+ 'authors_words_music': [], 'ccli_number': '',
620+ 'authors_none_label': 'Written by', 'title': 'My Song',
621+ 'authors_words_music_label': 'Words and Music',
622+ 'authors_none': ['my author'],
623+ 'ccli_license_label': 'CCLI License', 'authors_words': [],
624+ 'ccli_license': '0', 'authors_translation_label': 'Translation',
625+ 'authors_words_all': []}
626+ MockedRenderer.assert_called_once_with(**args)
627+ self.assertEqual(author_list, ['my author'],
628+ 'The author list should be returned correctly with one author')
629
630 def test_build_song_footer_two_authors(self):
631 """
632@@ -382,6 +421,7 @@
633 mock_author_song.author_type = AuthorType.Translation
634 mock_song.authors_songs.append(mock_author_song)
635 mock_song.copyright = 'My copyright'
636+ mock_song.songbook_entries = []
637 service_item = ServiceItem(None)
638
639 # WHEN: I generate the Footer with default settings
640@@ -389,7 +429,7 @@
641
642 # THEN: I get the following Array returned
643 assert service_item.raw_footer == ['My Song', 'Words: another author', 'Music: my author',
644- 'Translation: translator', 'My copyright'], \
645+ 'Translation: translator', '© My copyright'], \
646 'The array should be returned correctly with a song, two authors and copyright'
647 assert author_list == ['another author', 'my author', 'translator'], \
648 'The author list should be returned correctly with two authors'
649@@ -402,6 +442,7 @@
650 mock_song = MagicMock()
651 mock_song.title = 'My Song'
652 mock_song.copyright = 'My copyright'
653+ mock_song.songbook_entries = []
654 service_item = ServiceItem(None)
655 Settings().setValue('core/ccli number', '1234')
656
657@@ -409,7 +450,7 @@
658 self.media_item.generate_footer(service_item, mock_song)
659
660 # THEN: I get the following Array returned
661- assert service_item.raw_footer == ['My Song', 'My copyright', 'CCLI License: 1234'], \
662+ assert service_item.raw_footer == ['My Song', '© My copyright', 'CCLI License: 1234'], \
663 'The array should be returned correctly with a song, an author, copyright and ccli'
664
665 # WHEN: I amend the CCLI value
666@@ -417,7 +458,7 @@
667 self.media_item.generate_footer(service_item, mock_song)
668
669 # THEN: I would get an amended footer string
670- assert service_item.raw_footer == ['My Song', 'My copyright', 'CCLI License: 4321'], \
671+ assert service_item.raw_footer == ['My Song', '© My copyright', 'CCLI License: 4321'], \
672 'The array should be returned correctly with a song, an author, copyright and amended ccli'
673
674 def test_build_song_footer_base_songbook(self):
675@@ -431,6 +472,8 @@
676 song.copyright = 'My copyright'
677 song.authors_songs = []
678 song.songbook_entries = []
679+ song.alternate_title = ''
680+ song.topics = []
681 song.ccli_number = ''
682 book1 = MagicMock()
683 book1.name = 'My songbook'
684@@ -444,15 +487,8 @@
685 # WHEN: I generate the Footer with default settings
686 self.media_item.generate_footer(service_item, song)
687
688- # THEN: The songbook should not be in the footer
689- assert service_item.raw_footer == ['My Song', 'My copyright']
690-
691- # WHEN: I activate the "display songbook" option
692- self.media_item.display_songbook = True
693- self.media_item.generate_footer(service_item, song)
694-
695 # THEN: The songbook should be in the footer
696- assert service_item.raw_footer == ['My Song', 'My copyright', 'My songbook #12, Thy songbook #502A']
697+ assert service_item.raw_footer == ['My Song', '© My copyright', 'My songbook #12, Thy songbook #502A']
698
699 def test_build_song_footer_copyright_enabled(self):
700 """
701@@ -463,6 +499,7 @@
702 mock_song = MagicMock()
703 mock_song.title = 'My Song'
704 mock_song.copyright = 'My copyright'
705+ mock_song.songbook_entries = []
706 service_item = ServiceItem(None)
707
708 # WHEN: I generate the Footer with default settings
709@@ -479,13 +516,14 @@
710 mock_song = MagicMock()
711 mock_song.title = 'My Song'
712 mock_song.copyright = 'My copyright'
713+ mock_song.songbook_entries = []
714 service_item = ServiceItem(None)
715
716 # WHEN: I generate the Footer with default settings
717 self.media_item.generate_footer(service_item, mock_song)
718
719 # THEN: The copyright symbol should not be in the footer
720- assert service_item.raw_footer == ['My Song', 'My copyright']
721+ assert service_item.raw_footer == ['My Song', '© My copyright']
722
723 def test_authors_match(self):
724 """