openlp-core team mailing list archive

[Phill <phill.ridout@xxxxxxxxxxxxxx>]

Phill has proposed merging lp:~phill-ridout/openlp/171891 into lp:openlp.

Requested reviews:
  OpenLP Core (openlp-core)

For more details, see:
https://code.launchpad.net/~phill-ridout/openlp/171891/+merge/176456
-- 
https://code.launchpad.net/~phill-ridout/openlp/171891/+merge/176456
Your team OpenLP Core is requested to review the proposed merge of lp:~phill-ridout/openlp/171891 into lp:openlp.

=== modified file 'openlp/plugins/songs/lib/foilpresenterimport.py'
--- openlp/plugins/songs/lib/foilpresenterimport.py	2013-03-07 08:05:43 +0000
+++ openlp/plugins/songs/lib/foilpresenterimport.py	2013-07-23 18:39:25 +0000
@@ -96,6 +96,7 @@
 
 from lxml import etree, objectify
 
+from openlp.core.lib import translate
 from openlp.core.ui.wizard import WizardStrings
 from openlp.plugins.songs.lib import clean_song, VerseType
 from openlp.plugins.songs.lib.songimport import SongImport
@@ -115,7 +116,7 @@
         """
         log.debug(u'initialise FoilPresenterImport')
         SongImport.__init__(self, manager, **kwargs)
-        self.FoilPresenter = FoilPresenter(self.manager)
+        self.FoilPresenter = FoilPresenter(self.manager, self)
 
     def doImport(self):
         """
@@ -202,8 +203,9 @@
         <copyright> tag.
 
     """
-    def __init__(self, manager):
+    def __init__(self, manager, importer):
         self.manager = manager
+        self.importer = importer
 
     def xml_to_song(self, xml):
         """
@@ -222,22 +224,23 @@
         song.search_lyrics = u''
         song.verse_order = u''
         song.search_title = u''
-        # Because "text" seems to be an reserverd word, we have to recompile it.
+        self.skip_song = False
+        # Because "text" seems to be an reserved word, we have to recompile it.
         xml = re.compile(u'<text>').sub(u'<text_>', xml)
         xml = re.compile(u'</text>').sub(u'</text_>', xml)
         song_xml = objectify.fromstring(xml)
-        foilpresenterfolie = song_xml
-        self._process_copyright(foilpresenterfolie, song)
-        self._process_cclinumber(foilpresenterfolie, song)
-        self._process_titles(foilpresenterfolie, song)
+        self._process_copyright(song_xml, song)
+        self._process_cclinumber(song_xml, song)
+        self._process_titles(song_xml, song)
         # The verse order is processed with the lyrics!
-        self._process_lyrics(foilpresenterfolie, song)
-        self._process_comments(foilpresenterfolie, song)
-        self._process_authors(foilpresenterfolie, song)
-        self._process_songbooks(foilpresenterfolie, song)
-        self._process_topics(foilpresenterfolie, song)
-        clean_song(self.manager, song)
-        self.manager.save_object(song)
+        self._process_lyrics(song_xml, song)
+        self._process_comments(song_xml, song)
+        self._process_authors(song_xml, song)
+        self._process_songbooks(song_xml, song)
+        self._process_topics(song_xml, song)
+        if not self.skip_song:
+            clean_song(self.manager, song)
+            self.manager.save_object(song)
 
     def _child(self, element):
         """
@@ -420,6 +423,12 @@
             VerseType.tags[VerseType.Intro]: 1,
             VerseType.tags[VerseType.PreChorus]: 1
         }
+        if not hasattr(foilpresenterfolie.strophen, u'strophe'):
+            self.importer.logError(self._child(foilpresenterfolie.titel),
+                unicode(translate('SongsPlugin.FoilPresenterSongImport',
+                'Invalid Foilpresenter song file. No verses found.')))
+            self.skip_song = True
+            return
         for strophe in foilpresenterfolie.strophen.strophe:
             text = self._child(strophe.text_) if hasattr(strophe, u'text_') else u''
             verse_name = self._child(strophe.key)

=== added file 'tests/functional/openlp_plugins/songs/test_foilpresenterimport.py'
--- tests/functional/openlp_plugins/songs/test_foilpresenterimport.py	1970-01-01 00:00:00 +0000
+++ tests/functional/openlp_plugins/songs/test_foilpresenterimport.py	2013-07-23 18:39:25 +0000
@@ -0,0 +1,195 @@
+# -*- coding: utf-8 -*-
+# vim: autoindent shiftwidth=4 expandtab textwidth=120 tabstop=4 softtabstop=4
+
+###############################################################################
+# OpenLP - Open Source Lyrics Projection                                      #
+# --------------------------------------------------------------------------- #
+# Copyright (c) 2008-2013 Raoul Snyman                                        #
+# Portions copyright (c) 2008-2013 Tim Bentley, Gerald Britton, Jonathan      #
+# Corwin, Samuel Findlay, Michael Gorven, Scott Guerrieri, Matthias Hub,      #
+# Meinert Jordan, Armin Köhler, Erik Lundin, Edwin Lunando, Brian T. Meyer.   #
+# Joshua Miller, Stevan Pettit, Andreas Preikschat, Mattias Põldaru,          #
+# Christian Richter, Philip Ridout, Simon Scudder, Jeffrey Smith,             #
+# Maikel Stuivenberg, Martin Thompson, Jon Tibble, Dave Warnock,              #
+# Frode Woldsund, Martin Zibricky, Patrick Zimmermann                         #
+# --------------------------------------------------------------------------- #
+# This program is free software; you can redistribute it and/or modify it     #
+# under the terms of the GNU General Public License as published by the Free  #
+# Software Foundation; version 2 of the License.                              #
+#                                                                             #
+# This program is distributed in the hope that it will be useful, but WITHOUT #
+# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or       #
+# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for    #
+# more details.                                                               #
+#                                                                             #
+# You should have received a copy of the GNU General Public License along     #
+# with this program; if not, write to the Free Software Foundation, Inc., 59  #
+# Temple Place, Suite 330, Boston, MA 02111-1307 USA                          #
+###############################################################################
+"""
+This module contains tests for the SongShow Plus song importer.
+"""
+
+import os
+from unittest import TestCase
+from mock import patch, MagicMock
+
+from openlp.plugins.songs.lib import VerseType
+from openlp.plugins.songs.lib.foilpresenterimport import FoilPresenter
+
+TEST_PATH = os.path.abspath(
+    os.path.join(os.path.dirname(__file__), u'..', u'..', u'..', u'/resources/foilpresentersongs'))
+
+
+class TestFoilPresenter(TestCase):
+    """
+    Test the functions in the :mod:`foilpresenterimport` module.
+    """
+    #TODO: The following modules still need tests written for
+    #   xml_to_song
+    #   _child
+    #   _process_authors
+    #   _process_cclinumber
+    #   _process_comments
+    #   _process_copyright
+    #   _process_lyrics
+    #   _process_songbooks
+    #   _process_titles
+    #   _process_topics
+
+    def setUp(self):
+        self.child_patcher = patch(u'openlp.plugins.songs.lib.foilpresenterimport.FoilPresenter._child')
+        self.clean_song_patcher = patch(u'openlp.plugins.songs.lib.foilpresenterimport.clean_song')
+        self.objectify_patcher = patch(u'openlp.plugins.songs.lib.foilpresenterimport.objectify')
+        self.process_authors_patcher = \
+            patch(u'openlp.plugins.songs.lib.foilpresenterimport.FoilPresenter._process_authors')
+        self.process_cclinumber_patcher = \
+            patch(u'openlp.plugins.songs.lib.foilpresenterimport.FoilPresenter._process_cclinumber')
+        self.process_comments_patcher = \
+            patch(u'openlp.plugins.songs.lib.foilpresenterimport.FoilPresenter._process_comments')
+        self.process_lyrics_patcher = \
+            patch(u'openlp.plugins.songs.lib.foilpresenterimport.FoilPresenter._process_lyrics')
+        self.process_songbooks_patcher = \
+            patch(u'openlp.plugins.songs.lib.foilpresenterimport.FoilPresenter._process_songbooks')
+        self.process_titles_patcher = \
+            patch(u'openlp.plugins.songs.lib.foilpresenterimport.FoilPresenter._process_titles')
+        self.process_topics_patcher = \
+            patch(u'openlp.plugins.songs.lib.foilpresenterimport.FoilPresenter._process_topics')
+        self.re_patcher = patch(u'openlp.plugins.songs.lib.foilpresenterimport.re')
+        self.song_patcher = patch(u'openlp.plugins.songs.lib.foilpresenterimport.Song')
+        self.song_xml_patcher = patch(u'openlp.plugins.songs.lib.foilpresenterimport.SongXML')
+        self.translate_patcher = patch(u'openlp.plugins.songs.lib.foilpresenterimport.translate')
+
+        self.mocked_child = self.child_patcher.start()
+        self.mocked_clean_song = self.clean_song_patcher.start()
+        self.mocked_objectify = self.objectify_patcher.start()
+        self.mocked_process_authors = self.process_authors_patcher.start()
+        self.mocked_process_cclinumber = self.process_cclinumber_patcher.start()
+        self.mocked_process_comments = self.process_comments_patcher.start()
+        self.mocked_process_lyrics = self.process_lyrics_patcher.start()
+        self.mocked_process_songbooks = self.process_songbooks_patcher.start()
+        self.mocked_process_titles = self.process_titles_patcher.start()
+        self.mocked_process_topics = self.process_topics_patcher.start()
+        self.mocked_re = self.re_patcher.start()
+        self.mocked_song = self.song_patcher.start()
+        self.mocked_song_xml = self.song_xml_patcher.start()
+        self.mocked_translate = self.translate_patcher.start()
+        self.mocked_child.return_value = u'Element Text'
+        self.mocked_translate.return_value = u'Translated String'
+        self.mocked_manager = MagicMock()
+        self.mocked_song_import = MagicMock()
+
+    def tearDown(self):
+        self.child_patcher.stop()
+        self.clean_song_patcher.stop()
+        self.objectify_patcher.stop()
+        self.process_authors_patcher.stop()
+        self.process_cclinumber_patcher.stop()
+        self.process_comments_patcher.stop()
+        self.process_lyrics_patcher.stop()
+        self.process_songbooks_patcher.stop()
+        self.process_titles_patcher.stop()
+        self.process_topics_patcher.stop()
+        self.re_patcher.stop()
+        self.song_patcher.stop()
+        self.song_xml_patcher.stop()
+        self.translate_patcher.stop()
+
+    def create_foil_presenter_test(self):
+        """
+        Test creating an instance of the FoilPresenter class
+        """
+        # GIVEN: A mocked out "manager" and "SongImport" instance
+        mocked_manager = MagicMock()
+        mocked_song_import = MagicMock()
+
+        # WHEN: An FoilPresenter instance is created
+        foil_presenter_instance = FoilPresenter(mocked_manager, mocked_song_import)
+
+        # THEN: The instance should not be None
+        self.assertIsNotNone(foil_presenter_instance, u'FoilPresenter instance should not be none')
+
+    def no_xml_test(self):
+        """
+        Test calling xml_to_song with out the xml argument
+        """
+        # GIVEN: A mocked out "manager" and "SongImport" as well as an foil_presenter instance
+        mocked_manager = MagicMock()
+        mocked_song_import = MagicMock()
+        foil_presenter_instance = FoilPresenter(mocked_manager, mocked_song_import)
+
+        # WHEN: xml_to_song is called without valid an argument
+        for arg in [None, False, 0, u'']:
+            result = foil_presenter_instance.xml_to_song(arg)
+
+            # Then: xml_to_song should return False
+            self.assertEqual(result, None, u'xml_to_song should return None when called with %s' % arg)
+
+    def encoding_declaration_removal_test(self):
+        """
+        Test that the encoding declaration is removed
+        """
+        # GIVEN: A reset mocked out re and an instance of foil_presenter
+        self.mocked_re.reset()
+        foil_presenter_instance = FoilPresenter(self.mocked_manager, self.mocked_song_import)
+
+        # WHEN: xml_to_song is called with a string with an xml encoding declaration
+        foil_presenter_instance.xml_to_song(u'<?xml version="1.0" encoding="UTF-8"?>\n<foilpresenterfolie>')
+
+        # THEN: the xml encoding declaration should have been stripped
+        self.mocked_re.compile.sub.called_with(u'\n<foilpresenterfolie>')
+
+    def no_encoding_declaration_test(self):
+        """
+        Check that the xml sting is left intact when no encoding declaration is made
+        """
+        # GIVEN: A reset mocked out re and an instance of foil_presenter
+        self.mocked_re.reset()
+        foil_presenter_instance = FoilPresenter(self.mocked_manager, self.mocked_song_import)
+
+        # WHEN: xml_to_song is called with a string without an xml encoding declaration
+        foil_presenter_instance.xml_to_song(u'<foilpresenterfolie>')
+
+        # THEN: the string shiuld have been left intact
+        self.mocked_re.compile.sub.called_with(u'<foilpresenterfolie>')
+
+    def process_lyrics_no_verses_test(self):
+        """
+        Test that _process_lyrics handles song files that have no verses.
+        """
+        # GIVEN: A mocked foilpresenterfolie with no attribute strophe, a mocked song and a
+        #       foil presenter instance
+        self.process_lyrics_patcher.stop()
+        self.mocked_song_xml.reset()
+        mock_foilpresenterfolie = MagicMock()
+        del mock_foilpresenterfolie.strophen.strophe
+        mocked_song = MagicMock()
+        foil_presenter_instance = FoilPresenter(self.mocked_manager, self.mocked_song_import)
+
+        # WHEN: _process_lyrics is called
+        result = foil_presenter_instance._process_lyrics(mock_foilpresenterfolie, mocked_song)
+
+        # THEN: _process_lyrics should return None and the song_import logError method should have been called once
+        self.assertIsNone(result)
+        self.mocked_song_import.logError.assert_called_once_with(u'Element Text', u'Translated String')
+        self.process_lyrics_patcher.start()
\ No newline at end of file