openlp-core team mailing list archive

[Tomas Groth <tomasgroth@xxxxxxxx>]

Tomas Groth has proposed merging lp:~tomasgroth/openlp/zionworx-fix-22 into lp:openlp/2.2.

Requested reviews:
  OpenLP Core (openlp-core)

For more details, see:
https://code.launchpad.net/~tomasgroth/openlp/zionworx-fix-22/+merge/276061

Make the zionworx importer work under python3.
-- 
Your team OpenLP Core is requested to review the proposed merge of lp:~tomasgroth/openlp/zionworx-fix-22 into lp:openlp/2.2.

=== modified file 'openlp/plugins/songs/lib/importers/zionworx.py'
--- openlp/plugins/songs/lib/importers/zionworx.py	2015-03-09 20:57:39 +0000
+++ openlp/plugins/songs/lib/importers/zionworx.py	2015-10-28 21:55:40 +0000
@@ -75,7 +75,8 @@
         """
         Receive a CSV file (from a ZionWorx database dump) to import.
         """
-        with open(self.import_source, 'rb') as songs_file:
+        # Encoding should always be ISO-8859-1
+        with open(self.import_source, 'rt', encoding='ISO-8859-1') as songs_file:
             field_names = ['SongNum', 'Title1', 'Title2', 'Lyrics', 'Writer', 'Copyright', 'Keywords',
                            'DefaultStyle']
             songs_reader = csv.DictReader(songs_file, field_names)
@@ -112,10 +113,10 @@
                     if line and not line.isspace():
                         verse += line + '\n'
                     elif verse:
-                        self.add_verse(verse)
+                        self.add_verse(verse, 'v')
                         verse = ''
                 if verse:
-                    self.add_verse(verse)
+                    self.add_verse(verse, 'v')
                 title = self.title
                 if not self.finish():
                     self.log_error(translate('SongsPlugin.ZionWorxImport', 'Record %d') % index +
@@ -123,8 +124,7 @@
 
     def _decode(self, str):
         """
-        Decodes CSV input to unicode, stripping all control characters (except new lines).
+        Strips all control characters (except new lines).
         """
-        # This encoding choice seems OK. ZionWorx has no option for setting the
-        # encoding for its songs, so we assume encoding is always the same.
-        return str(str, 'cp1252').translate(CONTROL_CHARS_MAP)
+        # ZionWorx has no option for setting the encoding for its songs, so we assume encoding is always the same.
+        return str.translate(CONTROL_CHARS_MAP)

=== modified file 'tests/functional/openlp_plugins/songs/test_zionworximport.py'
--- tests/functional/openlp_plugins/songs/test_zionworximport.py	2015-01-18 13:39:21 +0000
+++ tests/functional/openlp_plugins/songs/test_zionworximport.py	2015-10-28 21:55:40 +0000
@@ -22,14 +22,19 @@
 """
 This module contains tests for the ZionWorx song importer.
 """
+import os
 
 from unittest import TestCase
 
 from tests.functional import MagicMock, patch
+from tests.helpers.songfileimport import SongImportTestHelper
 from openlp.plugins.songs.lib.importers.zionworx import ZionWorxImport
 from openlp.plugins.songs.lib.importers.songimport import SongImport
 from openlp.core.common import Registry
 
+TEST_PATH = os.path.abspath(
+    os.path.join(os.path.dirname(__file__), '..', '..', '..', 'resources', 'zionworxsongs'))
+
 
 class TestZionWorxImport(TestCase):
     """
@@ -54,3 +59,18 @@
 
             # THEN: The importer should be an instance of SongImport
             self.assertIsInstance(importer, SongImport)
+
+
+class TestZionWorxFileImport(SongImportTestHelper):
+
+    def __init__(self, *args, **kwargs):
+        self.importer_class_name = 'ZionWorxImport'
+        self.importer_module_name = 'zionworx'
+        super(TestZionWorxFileImport, self).__init__(*args, **kwargs)
+
+    def test_song_import(self):
+        """
+        Test that loading an ZionWorx file works correctly on various files
+        """
+        self.file_import(os.path.join(TEST_PATH, 'zionworx.csv'),
+                         self.load_external_result_data(os.path.join(TEST_PATH, 'zionworx.json')))

=== added directory 'tests/resources/zionworxsongs'
=== added file 'tests/resources/zionworxsongs/zionworx.csv'
--- tests/resources/zionworxsongs/zionworx.csv	1970-01-01 00:00:00 +0000
+++ tests/resources/zionworxsongs/zionworx.csv	2015-10-28 21:55:40 +0000
@@ -0,0 +1,45 @@
+"1","Crown Him With Many Crowns",,"Crown him with many crowns,
+The Lamb upon His throne;
+Hark, how the heavenly anthem drowns
+All music but its own!
+Awake, my soul, and sing
+Of Him who died for thee,
+And hail Him as thy matchless King
+Through all eternity.
+
+Crown Him the Lord of life,
+Who triumphed o'er the grave
+And rose victorious in the strife
+For those He came to save:
+His glories now we sing,
+Who died and rose on high,
+Who died eternal life to bring
+And lives that death may die.
+
+Crown Him the Lord of love;
+Behold His hands and side,
+Those wounds yet visible above
+In beauty glorified:
+No angel in the sky
+Can fully bear that sight,
+But downward bends His burning eye
+At mysteries so bright.
+
+Crown Him the Lord of peace,
+Whose power a sceptre sways
+From pole to pole, that wars may cease,
+And all be prayer and praise:
+His reign shall know no end,
+And round His pierc�feet
+Fair flowers of paradise extend
+Their fragrance ever sweet.
+
+Crown Him the Lord of years,
+The Potentate of time,
+Creator of the rolling spheres,
+Ineffably sublime!
+All hail, Redeemer, hail!
+For Thou hast died for me;
+Thy praise shall never, never fail
+Throughout eternity.
+","Matthew Bridges","Public Domain",,

=== added file 'tests/resources/zionworxsongs/zionworx.json'
--- tests/resources/zionworxsongs/zionworx.json	1970-01-01 00:00:00 +0000
+++ tests/resources/zionworxsongs/zionworx.json	2015-10-28 21:55:40 +0000
@@ -0,0 +1,30 @@
+{
+    "authors": [
+        "Matthew Bridges"
+    ],
+    "copyright": "Public Domain",
+    "title": "Crown Him With Many Crowns",
+    "verse_order_list": [],
+    "verses": [
+        [
+            "Crown him with many crowns,\nThe Lamb upon His throne;\nHark, how the heavenly anthem drowns\nAll music but its own!\nAwake, my soul, and sing\nOf Him who died for thee,\nAnd hail Him as thy matchless King\nThrough all eternity.\n",
+            "v"
+        ],
+        [
+            "Crown Him the Lord of life,\nWho triumphed o'er the grave\nAnd rose victorious in the strife\nFor those He came to save:\nHis glories now we sing,\nWho died and rose on high,\nWho died eternal life to bring\nAnd lives that death may die.\n",
+            "v"
+        ],
+        [
+            "Crown Him the Lord of love;\nBehold His hands and side,\nThose wounds yet visible above\nIn beauty glorified:\nNo angel in the sky\nCan fully bear that sight,\nBut downward bends His burning eye\nAt mysteries so bright.\n",
+            "v"
+        ],
+        [
+            "Crown Him the Lord of peace,\nWhose power a sceptre sways\nFrom pole to pole, that wars may cease,\nAnd all be prayer and praise:\nHis reign shall know no end,\nAnd round His piercèd feet\nFair flowers of paradise extend\nTheir fragrance ever sweet.\n",
+            "v"
+        ],
+        [
+            "Crown Him the Lord of years,\nThe Potentate of time,\nCreator of the rolling spheres,\nIneffably sublime!\nAll hail, Redeemer, hail!\nFor Thou hast died for me;\nThy praise shall never, never fail\nThroughout eternity.\n",
+            "v"
+        ]
+    ]
+}