openlp-core team mailing list archive
-
openlp-core team
-
Mailing list archive
-
Message #20587
[Merge] lp:~trb143/openlp/cleanup into lp:openlp
Tim Bentley has proposed merging lp:~trb143/openlp/cleanup into lp:openlp.
Requested reviews:
Raoul Snyman (raoul-snyman)
Related bugs:
Bug #1090641 in OpenLP: "CCLI license doesn't display"
https://bugs.launchpad.net/openlp/+bug/1090641
For more details, see:
https://code.launchpad.net/~trb143/openlp/cleanup/+merge/160474
Change generation of Song Footer to song load code will regenerate footer and get correct CCLI.
Clean up code so call paths are now logical
Some minor code cleanups.
Move some code from the constructor as it was in the wrong place.
Create so test cases for the song footer generation.
--
https://code.launchpad.net/~trb143/openlp/cleanup/+merge/160474
Your team OpenLP Core is subscribed to branch lp:openlp.
=== modified file 'openlp/core/lib/mediamanageritem.py'
--- openlp/core/lib/mediamanageritem.py 2013-04-14 15:59:57 +0000
+++ openlp/core/lib/mediamanageritem.py 2013-04-23 19:11:26 +0000
@@ -102,7 +102,6 @@
self.setupUi()
self.retranslateUi()
self.auto_select_id = -1
- Registry().register_function(u'%s_service_load' % self.plugin.name, self.service_load)
# Need to use event as called across threads and UI is updated
QtCore.QObject.connect(self, QtCore.SIGNAL(u'%s_go_live' % self.plugin.name), self.go_live_remote)
QtCore.QObject.connect(self, QtCore.SIGNAL(u'%s_add_to_service' % self.plugin.name), self.add_to_service_remote)
@@ -585,12 +584,15 @@
else:
return None
- def service_load(self, message):
+ def service_load(self, item):
"""
Method to add processing when a service has been loaded and individual service items need to be processed by the
plugins.
+
+ ``item``
+ The item to be processed and returned.
"""
- pass
+ return item
def check_search_result(self):
"""
=== modified file 'openlp/core/ui/servicemanager.py'
--- openlp/core/ui/servicemanager.py 2013-04-05 19:37:56 +0000
+++ openlp/core/ui/servicemanager.py 2013-04-23 19:11:26 +0000
@@ -715,13 +715,10 @@
else:
service_item.set_from_service(item, self.servicePath)
service_item.validate_item(self.suffixes)
- self.load_item_unique_identifier = 0
if service_item.is_capable(ItemCapabilities.OnLoadUpdate):
- Registry().execute(u'%s_service_load' % service_item.name.lower(), service_item)
- # if the item has been processed
- if service_item.unique_identifier == self.load_item_unique_identifier:
- service_item.edit_id = int(self.load_item_edit_id)
- service_item.temporary_edit = self.load_item_temporary
+ new_item = Registry().get(service_item.name).service_load(service_item)
+ if new_item:
+ service_item = new_item
self.add_service_item(service_item, repaint=False)
delete_file(p_file)
self.main_window.add_recent_file(file_name)
@@ -1260,14 +1257,6 @@
self.repaint_service_list(-1, -1)
self.application.set_normal_cursor()
- def service_item_update(self, edit_id, unique_identifier, temporary=False):
- """
- Triggered from plugins to update service items. Save the values as they will be used as part of the service load
- """
- self.load_item_unique_identifier = unique_identifier
- self.load_item_edit_id = int(edit_id)
- self.load_item_temporary = str_to_bool(temporary)
-
def replace_service_item(self, newItem):
"""
Using the service item passed replace the one with the same edit id if found.
=== modified file 'openlp/plugins/custom/lib/mediaitem.py'
--- openlp/plugins/custom/lib/mediaitem.py 2013-03-29 08:25:33 +0000
+++ openlp/plugins/custom/lib/mediaitem.py 2013-04-23 19:11:26 +0000
@@ -40,6 +40,7 @@
log = logging.getLogger(__name__)
+
class CustomSearch(object):
"""
An enumeration for custom search methods.
@@ -216,7 +217,6 @@
Settings().setValue(u'%s/last search type' % self.settings_section, self.search_text_edit.current_search_type())
# Reload the list considering the new search type.
search_keywords = self.search_text_edit.displayText()
- search_results = []
search_type = self.search_text_edit.current_search_type()
if search_type == CustomSearch.Titles:
log.debug(u'Titles Search')
@@ -255,7 +255,8 @@
and_(CustomSlide.title == item.title, CustomSlide.theme_name == item.theme,
CustomSlide.credits == item.raw_footer[0][len(item.title) + 1:]))
if custom:
- self.service_manager.service_item_update(custom.id, item.unique_identifier)
+ item.edit_id = custom.id
+ return item
else:
if self.add_custom_from_service:
self.create_from_service_item(item)
@@ -284,8 +285,6 @@
custom.text = unicode(custom_xml.extract_xml(), u'utf-8')
self.plugin.manager.save_object(custom)
self.on_search_text_button_clicked()
- if item.name.lower() == u'custom':
- Registry().execute(u'service_item_update', u'%s:%s:%s' % (custom.id, item.unique_identifier, False))
def onClearTextButtonClick(self):
"""
=== modified file 'openlp/plugins/songs/forms/__init__.py'
--- openlp/plugins/songs/forms/__init__.py 2013-03-07 21:04:19 +0000
+++ openlp/plugins/songs/forms/__init__.py 2013-04-23 19:11:26 +0000
@@ -52,3 +52,4 @@
them separate from the functionality, so that it is easier to recreate the GUI
from the .ui files later if necessary.
"""
+from editsongform import EditSongForm
\ No newline at end of file
=== modified file 'openlp/plugins/songs/lib/mediaitem.py'
--- openlp/plugins/songs/lib/mediaitem.py 2013-04-20 20:19:53 +0000
+++ openlp/plugins/songs/lib/mediaitem.py 2013-04-23 19:11:26 +0000
@@ -72,10 +72,7 @@
def __init__(self, parent, plugin):
self.icon_path = u'songs/song'
MediaManagerItem.__init__(self, parent, plugin)
- self.editSongForm = EditSongForm(self, self.main_window, self.plugin.manager)
- self.openLyrics = OpenLyrics(self.plugin.manager)
self.single_service_item = False
- self.songMaintenanceForm = SongMaintenanceForm(self.plugin.manager, self)
# Holds information about whether the edit is remotely triggered and
# which Song is required.
self.remoteSong = -1
@@ -135,6 +132,12 @@
'Maintain the lists of authors, topics and books.'))
def initialise(self):
+ """
+ Initialise variables when they cannot be initialised in the constructor.
+ """
+ self.songMaintenanceForm = SongMaintenanceForm(self.plugin.manager, self)
+ self.editSongForm = EditSongForm(self, self.main_window, self.plugin.manager)
+ self.openLyrics = OpenLyrics(self.plugin.manager)
self.search_text_edit.set_search_types([
(SongSearch.Entire, u':/songs/song_search_all.png',
translate('SongsPlugin.MediaItem', 'Entire Song'),
@@ -160,7 +163,6 @@
Settings().setValue(u'%s/last search type' % self.settings_section, self.search_text_edit.current_search_type())
# Reload the list considering the new search type.
search_keywords = unicode(self.search_text_edit.displayText())
- search_results = []
search_type = self.search_text_edit.current_search_type()
if search_type == SongSearch.Entire:
log.debug(u'Entire Song Search')
@@ -463,16 +465,7 @@
for slide in verses:
service_item.add_from_text(unicode(slide))
service_item.title = song.title
- author_list = [unicode(author.display_name) for author in song.authors]
- service_item.raw_footer.append(song.title)
- service_item.raw_footer.append(create_separated_list(author_list))
- service_item.raw_footer.append(song.copyright)
- if Settings().value(u'core/ccli number'):
- service_item.raw_footer.append(translate('SongsPlugin.MediaItem', 'CCLI License: ') +
- Settings().value(u'core/ccli number'))
- service_item.audit = [
- song.title, author_list, song.copyright, unicode(song.ccli_number)
- ]
+ author_list = self.generate_footer(service_item, song)
service_item.data_string = {u'title': song.search_title, u'authors': u', '.join(author_list)}
service_item.xml_version = self.openLyrics.song_to_xml(song)
# Add the audio file to the service item.
@@ -481,6 +474,30 @@
service_item.background_audio = [m.file_name for m in song.media_files]
return True
+ def generate_footer(self, item, song):
+ """
+ Generates the song footer based on a song and adds details to a service item.
+ author_list is only required for initial song generation.
+
+ ``item``
+ The service item to be amended
+
+ ``song``
+ The song to be used to generate the footer
+ """
+ author_list = [unicode(author.display_name) for author in song.authors]
+ item.audit = [
+ song.title, author_list, song.copyright, unicode(song.ccli_number)
+ ]
+ item.raw_footer = []
+ item.raw_footer.append(song.title)
+ item.raw_footer.append(create_separated_list(author_list))
+ item.raw_footer.append(song.copyright)
+ if Settings().value(u'core/ccli number'):
+ item.raw_footer.append(translate('SongsPlugin.MediaItem', 'CCLI License: ') +
+ Settings().value(u'core/ccli number'))
+ return author_list
+
def service_load(self, item):
"""
Triggered by a song being loaded by the service manager.
@@ -499,9 +516,8 @@
else:
search_results = self.plugin.manager.get_all_objects(Song,
Song.search_title == item.data_string[u'title'], Song.search_title.asc())
- editId = 0
+ edit_id = 0
add_song = True
- temporary = False
if search_results:
for song in search_results:
author_list = item.data_string[u'authors']
@@ -514,7 +530,7 @@
break
if same_authors and author_list.strip(u', ') == u'':
add_song = False
- editId = song.id
+ edit_id = song.id
break
# If there's any backing tracks, copy them over.
if item.background_audio:
@@ -524,7 +540,7 @@
# If there's any backing tracks, copy them over.
if item.background_audio:
self._updateBackgroundAudio(song, item)
- editId = song.id
+ edit_id = song.id
self.on_search_text_button_clicked()
elif add_song and not self.addSongFromService:
# Make sure we temporary import formatting tags.
@@ -532,11 +548,11 @@
# If there's any backing tracks, copy them over.
if item.background_audio:
self._updateBackgroundAudio(song, item)
- editId = song.id
- temporary = True
- # Update service with correct song id.
- if editId:
- self.service_manager.service_item_update(editId, item.unique_identifier, temporary)
+ edit_id = song.id
+ # Update service with correct song id and return it to caller.
+ item.edit_id = edit_id
+ self.generate_footer(item, song)
+ return item
def search(self, string, showError):
"""
=== modified file 'openlp/plugins/songs/songsplugin.py'
--- openlp/plugins/songs/songsplugin.py 2013-03-19 22:00:50 +0000
+++ openlp/plugins/songs/songsplugin.py 2013-04-23 19:11:26 +0000
@@ -235,8 +235,7 @@
u'delete': translate('SongsPlugin', 'Delete the selected song.'),
u'preview': translate('SongsPlugin', 'Preview the selected song.'),
u'live': translate('SongsPlugin', 'Send the selected song live.'),
- u'service': translate('SongsPlugin',
- 'Add the selected song to the service.')
+ u'service': translate('SongsPlugin', 'Add the selected song to the service.')
}
self.set_plugin_ui_text_strings(tooltips)
=== modified file 'tests/functional/openlp_core_lib/test_screen.py'
--- tests/functional/openlp_core_lib/test_screen.py 2013-03-07 12:34:35 +0000
+++ tests/functional/openlp_core_lib/test_screen.py 2013-04-23 19:11:26 +0000
@@ -1,7 +1,7 @@
"""
Package to test the openlp.core.lib.screenlist package.
"""
-import copy
+
from unittest import TestCase
from mock import MagicMock
=== added file 'tests/functional/openlp_plugins/songs/test_mediaitem.py'
--- tests/functional/openlp_plugins/songs/test_mediaitem.py 1970-01-01 00:00:00 +0000
+++ tests/functional/openlp_plugins/songs/test_mediaitem.py 2013-04-23 19:11:26 +0000
@@ -0,0 +1,125 @@
+"""
+This module contains tests for the lib submodule of the Songs plugin.
+"""
+import os
+from tempfile import mkstemp
+from unittest import TestCase
+
+from mock import patch, MagicMock
+
+from PyQt4 import QtGui
+
+from openlp.core.lib import Registry, ServiceItem, Settings
+
+from openlp.plugins.songs.lib.mediaitem import SongMediaItem
+
+
+class TestMediaItem(TestCase):
+ """
+ Test the functions in the :mod:`lib` module.
+ """
+ def setUp(self):
+ """
+ Set up the components need for all tests.
+ """
+ Registry.create()
+ Registry().register(u'service_list', MagicMock())
+
+ with patch('openlp.core.lib.mediamanageritem.MediaManagerItem.__init__'), \
+ patch('openlp.plugins.songs.forms.editsongform.EditSongForm.__init__') :
+ self.media_item = SongMediaItem(MagicMock(), MagicMock())
+
+ fd, self.ini_file = mkstemp(u'.ini')
+ Settings().set_filename(self.ini_file)
+ self.application = QtGui.QApplication.instance()
+
+ def tearDown(self):
+ """
+ Delete all the C++ objects at the end so that we don't have a segfault
+ """
+ del self.application
+ # Not all tests use settings!
+ try:
+ os.unlink(self.ini_file)
+ os.unlink(Settings().fileName())
+ except:
+ pass
+
+ def build_song_footer_one_author_test(self):
+ """
+ Test build songs footer with basic song and one author
+ """
+ # GIVEN: A Song and a Service Item
+ mock_song = MagicMock()
+ mock_song.title = u'My Song'
+ mock_author = MagicMock()
+ mock_author.display_name = u'my author'
+ mock_song.authors = []
+ mock_song.authors.append(mock_author)
+ mock_song.copyright = u'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, [u'My Song', u'my author', u'My copyright'],
+ u'The array should be returned correctly with a song, one author and copyright')
+ self.assertEqual(author_list, [u'my author'],
+ u'The author list should be returned correctly with one author')
+
+ def build_song_footer_two_authors_test(self):
+ """
+ Test build songs footer with basic song and two authors
+ """
+ # GIVEN: A Song and a Service Item
+ mock_song = MagicMock()
+ mock_song.title = u'My Song'
+ mock_author = MagicMock()
+ mock_author.display_name = u'my author'
+ mock_song.authors = []
+ mock_song.authors.append(mock_author)
+ mock_author = MagicMock()
+ mock_author.display_name = u'another author'
+ mock_song.authors.append(mock_author)
+ mock_song.copyright = u'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, [u'My Song', u'my author and another author', u'My copyright'],
+ u'The array should be returned correctly with a song, two authors and copyright')
+ self.assertEqual(author_list, [u'my author', u'another author'],
+ u'The author list should be returned correctly with two authors')
+
+ def build_song_footer_base_ccli_test(self):
+ """
+ Test build songs footer with basic song and two authors
+ """
+ # GIVEN: A Song and a Service Item and a configured CCLI license
+ mock_song = MagicMock()
+ mock_song.title = u'My Song'
+ mock_author = MagicMock()
+ mock_author.display_name = u'my author'
+ mock_song.authors = []
+ mock_song.authors.append(mock_author)
+ mock_song.copyright = u'My copyright'
+ service_item = ServiceItem(None)
+ Settings().setValue(u'core/ccli number', u'1234')
+
+ # WHEN: I generate the Footer with default settings
+ self.media_item.generate_footer(service_item, mock_song)
+
+ # THEN: I get the following Array returned
+ self.assertEqual(service_item.raw_footer, [u'My Song', u'my author', u'My copyright', u'CCLI License: 1234'],
+ u'The array should be returned correctly with a song, an author, copyright and ccli')
+
+ # WHEN: I amend the CCLI value
+ Settings().setValue(u'core/ccli number', u'4321')
+ self.media_item.generate_footer(service_item, mock_song)
+
+ # THEN: I would get an amended footer string
+ self.assertEqual(service_item.raw_footer, [u'My Song', u'my author', u'My copyright', u'CCLI License: 4321'],
+ u'The array should be returned correctly with a song, an author, copyright and amended ccli')
\ No newline at end of file
