openlp-core team mailing list archive

[Tim Bentley <tim.bentley@xxxxxxxxx>]

Tim Bentley has proposed merging lp:~trb143/openlp/cleanup into lp:openlp.

Requested reviews:
  OpenLP Core (openlp-core)
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/159980

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.
-- 
https://code.launchpad.net/~trb143/openlp/cleanup/+merge/159980
Your team OpenLP Core is requested to review the proposed merge of lp:~trb143/openlp/cleanup into 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-21 15:57:25 +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-21 15:57:25 +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-21 15:57:25 +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/lib/mediaitem.py'
--- openlp/plugins/songs/lib/mediaitem.py	2013-04-20 20:19:53 +0000
+++ openlp/plugins/songs/lib/mediaitem.py	2013-04-21 15:57:25 +0000
@@ -160,7 +160,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 +462,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 +471,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 +513,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 +527,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 +537,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 +545,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):
         """