openlp-core team mailing list archive

[Simon Hanna <simon.hanna@xxxxxxxxxxxxx>]

Simon Hanna has proposed merging lp:~thelinuxguy/openlp/fix-song-import into lp:openlp.

Requested reviews:
  OpenLP Core (openlp-core)

For more details, see:
https://code.launchpad.net/~thelinuxguy/openlp/fix-song-import/+merge/282108

Fix SongImport
Ignore Case for versetags when importing Songbeamer files

The issue with importing songs was because the SongFormat list was reordered, but the ordering wasn't passed through to get_format_list()
This error should not happen again, added a testcase just to be sure
-- 
Your team OpenLP Core is requested to review the proposed merge of lp:~thelinuxguy/openlp/fix-song-import into lp:openlp.

=== modified file 'openlp/plugins/songs/lib/importer.py'
--- openlp/plugins/songs/lib/importer.py	2016-01-08 19:53:58 +0000
+++ openlp/plugins/songs/lib/importer.py	2016-01-10 00:38:16 +0000
@@ -390,7 +390,7 @@
         """
         Return a list of the supported song formats.
         """
-        return [
+        return sorted([
             SongFormat.OpenLyrics,
             SongFormat.OpenLP2,
             SongFormat.Generic,
@@ -400,6 +400,7 @@
             SongFormat.EasyWorshipDB,
             SongFormat.EasyWorshipService,
             SongFormat.FoilPresenter,
+            SongFormat.Lyrix,
             SongFormat.MediaShout,
             SongFormat.OpenSong,
             SongFormat.PowerPraise,
@@ -411,13 +412,12 @@
             SongFormat.SongShowPlus,
             SongFormat.SongsOfFellowship,
             SongFormat.SundayPlus,
+            SongFormat.VideoPsalm,
             SongFormat.WordsOfWorship,
             SongFormat.WorshipAssistant,
             SongFormat.WorshipCenterPro,
             SongFormat.ZionWorx,
-            SongFormat.Lyrix,
-            SongFormat.VideoPsalm
-        ]
+        ])
 
     @staticmethod
     def get(song_format, *attributes):

=== modified file 'openlp/plugins/songs/lib/importers/songbeamer.py'
--- openlp/plugins/songs/lib/importers/songbeamer.py	2016-01-03 00:39:53 +0000
+++ openlp/plugins/songs/lib/importers/songbeamer.py	2016-01-10 00:38:16 +0000
@@ -36,28 +36,28 @@
 
 class SongBeamerTypes(object):
     MarkTypes = {
-        'Refrain': VerseType.tags[VerseType.Chorus],
-        'Chorus': VerseType.tags[VerseType.Chorus],
-        'Vers': VerseType.tags[VerseType.Verse],
-        'Verse': VerseType.tags[VerseType.Verse],
-        'Strophe': VerseType.tags[VerseType.Verse],
-        'Intro': VerseType.tags[VerseType.Intro],
-        'Coda': VerseType.tags[VerseType.Ending],
-        'Ending': VerseType.tags[VerseType.Ending],
-        'Bridge': VerseType.tags[VerseType.Bridge],
-        'Interlude': VerseType.tags[VerseType.Bridge],
-        'Zwischenspiel': VerseType.tags[VerseType.Bridge],
-        'Pre-Chorus': VerseType.tags[VerseType.PreChorus],
-        'Pre-Refrain': VerseType.tags[VerseType.PreChorus],
-        'Misc': VerseType.tags[VerseType.Other],
-        'Pre-Bridge': VerseType.tags[VerseType.Other],
-        'Pre-Coda': VerseType.tags[VerseType.Other],
-        'Part': VerseType.tags[VerseType.Other],
-        'Teil': VerseType.tags[VerseType.Other],
-        'Unbekannt': VerseType.tags[VerseType.Other],
-        'Unknown': VerseType.tags[VerseType.Other],
-        'Unbenannt': VerseType.tags[VerseType.Other],
-        '$$M=': VerseType.tags[VerseType.Other]
+        'refrain': VerseType.tags[VerseType.Chorus],
+        'chorus': VerseType.tags[VerseType.Chorus],
+        'vers': VerseType.tags[VerseType.Verse],
+        'verse': VerseType.tags[VerseType.Verse],
+        'strophe': VerseType.tags[VerseType.Verse],
+        'intro': VerseType.tags[VerseType.Intro],
+        'coda': VerseType.tags[VerseType.Ending],
+        'ending': VerseType.tags[VerseType.Ending],
+        'bridge': VerseType.tags[VerseType.Bridge],
+        'interlude': VerseType.tags[VerseType.Bridge],
+        'zwischenspiel': VerseType.tags[VerseType.Bridge],
+        'pre-chorus': VerseType.tags[VerseType.PreChorus],
+        'pre-refrain': VerseType.tags[VerseType.PreChorus],
+        'misc': VerseType.tags[VerseType.Other],
+        'pre-bridge': VerseType.tags[VerseType.Other],
+        'pre-coda': VerseType.tags[VerseType.Other],
+        'part': VerseType.tags[VerseType.Other],
+        'teil': VerseType.tags[VerseType.Other],
+        'unbekannt': VerseType.tags[VerseType.Other],
+        'unknown': VerseType.tags[VerseType.Other],
+        'unbenannt': VerseType.tags[VerseType.Other],
+        '$$m=': VerseType.tags[VerseType.Other]
     }
 
 
@@ -267,20 +267,20 @@
 
     def check_verse_marks(self, line):
         """
-        Check and add the verse's MarkType. Returns ``True`` if the given linE contains a correct verse mark otherwise
+        Check and add the verse's MarkType. Returns ``True`` if the given line contains a correct verse mark otherwise
         ``False``.
 
         :param line: The line to check for marks (unicode).
         """
         marks = line.split(' ')
-        if len(marks) <= 2 and marks[0] in SongBeamerTypes.MarkTypes:
-            self.current_verse_type = SongBeamerTypes.MarkTypes[marks[0]]
+        if len(marks) <= 2 and marks[0].lower() in SongBeamerTypes.MarkTypes:
+            self.current_verse_type = SongBeamerTypes.MarkTypes[marks[0].lower()]
             if len(marks) == 2:
                 # If we have a digit, we append it to current_verse_type.
                 if marks[1].isdigit():
                     self.current_verse_type += marks[1]
             return True
-        elif marks[0].startswith('$$M='):  # this verse-mark cannot be numbered
-            self.current_verse_type = SongBeamerTypes.MarkTypes['$$M=']
+        elif marks[0].lower().startswith('$$m='):  # this verse-mark cannot be numbered
+            self.current_verse_type = SongBeamerTypes.MarkTypes['$$m=']
             return True
         return False

=== modified file 'tests/functional/openlp_plugins/songs/test_songbeamerimport.py'
--- tests/functional/openlp_plugins/songs/test_songbeamerimport.py	2015-12-31 22:46:06 +0000
+++ tests/functional/openlp_plugins/songs/test_songbeamerimport.py	2016-01-10 00:38:16 +0000
@@ -28,7 +28,7 @@
 
 from tests.helpers.songfileimport import SongImportTestHelper
 from tests.functional import MagicMock, patch
-from openlp.plugins.songs.lib.importers.songbeamer import SongBeamerImport
+from openlp.plugins.songs.lib.importers.songbeamer import SongBeamerImport, SongBeamerTypes
 from openlp.core.common import Registry
 
 TEST_PATH = os.path.abspath(os.path.join(os.path.dirname(__file__),
@@ -131,22 +131,22 @@
         self.assertEqual(self.current_verse_type, 'c', '<Refrain> should be interpreted as <c>')
 
         # GIVEN: line with unnumbered verse-type and trailing space
-        line = 'Refrain '
+        line = 'ReFrain '
         self.current_verse_type = None
         # WHEN: line is being checked for verse marks
         result = SongBeamerImport.check_verse_marks(self, line)
         # THEN: we should get back true and c as self.current_verse_type
-        self.assertTrue(result, 'Versemark for <Refrain > should be found, value true')
-        self.assertEqual(self.current_verse_type, 'c', '<Refrain > should be interpreted as <c>')
+        self.assertTrue(result, 'Versemark for <ReFrain > should be found, value true')
+        self.assertEqual(self.current_verse_type, 'c', '<ReFrain > should be interpreted as <c>')
 
         # GIVEN: line with numbered verse-type
-        line = 'Verse 1'
+        line = 'VersE 1'
         self.current_verse_type = None
         # WHEN: line is being checked for verse marks
         result = SongBeamerImport.check_verse_marks(self, line)
         # THEN: we should get back true and v1 as self.current_verse_type
-        self.assertTrue(result, 'Versemark for <Verse 1> should be found, value true')
-        self.assertEqual(self.current_verse_type, 'v1', u'<Verse 1> should be interpreted as <v1>')
+        self.assertTrue(result, 'Versemark for <VersE 1> should be found, value true')
+        self.assertEqual(self.current_verse_type, 'v1', u'<VersE 1> should be interpreted as <v1>')
 
         # GIVEN: line with special unnumbered verse-mark (used in Songbeamer to allow usage of non-supported tags)
         line = '$$M=special'
@@ -192,3 +192,12 @@
         # THEN: we should get back false and none as self.current_verse_type
         self.assertFalse(result, 'No versemark for <> should be found, value false')
         self.assertIsNone(self.current_verse_type, '<> should be interpreted as none versemark')
+
+    def test_verse_marks_defined_in_lowercase(self):
+        """
+        Test that the verse marks are all defined in lowercase
+        """
+        # GIVEN: SongBeamber MarkTypes
+        for tag in SongBeamerTypes.MarkTypes.keys():
+            # THEN: tag should be defined in lowercase
+            self.assertEquals(tag, tag.lower(), 'Tags should be defined in lowercase')

=== modified file 'tests/functional/openlp_plugins/songs/test_songformat.py'
--- tests/functional/openlp_plugins/songs/test_songformat.py	2016-01-09 09:09:29 +0000
+++ tests/functional/openlp_plugins/songs/test_songformat.py	2016-01-10 00:38:16 +0000
@@ -81,3 +81,14 @@
             # THEN: Return all attributes that were specified
             self.assertEquals(len(SongFormat.get(song_format, 'canDisable', 'availability')), 2,
                     "Did not return the correct number of attributes when retrieving multiple attributes at once")
+
+    def test_get_format_list_returns_ordered_list(self):
+        """
+        Test that get_format_list() returns a list that is ordered
+        according to the order specified in SongFormat
+        """
+        # GIVEN: The SongFormat class
+        # WHEN: Retrieving all formats
+        # THEN: The returned list should be sorted according to the ordering defined in SongFormat
+        self.assertEquals(sorted(SongFormat.get_format_list()), SongFormat.get_format_list(),
+                          "The list returned should be sorted according to the ordering in SongFormat")