Fix for bug #1387278: Crash when exporting settings

Fix for bug #1390917: Traceback when copying or creating a theme with background image on Windows
Fix for bug #1390986: Worship Assistent import gives traceback, also improved WA verseorder import
Fix for bug #1390987: Traceback when previewing item from service list
Fix traceback when saving service while in debug mode

bzr-revno: 2446

openlp/core/ui/mainwindow.py

@@ -978,7 +978,14 @@ class MainWindow(QtGui.QMainWindow, Ui_MainWindow, RegistryProperties):
             # FIXME: We are conflicting with the standard "General" section.
             if 'eneral' in section_key:
                 section_key = section_key.lower()
-            key_value = settings.value(section_key)
+            try:
+                key_value = settings.value(section_key)
+            except KeyError:
+                QtGui.QMessageBox.critical(self, translate('OpenLP.MainWindow', 'Export setting error'),
+                                           translate('OpenLP.MainWindow', 'The key "%s" does not have a default value '
+                                                     'so it will be skipped in this export.') % section_key,
+                                           QtGui.QMessageBox.StandardButtons(QtGui.QMessageBox.Ok))
+                key_value = None
             if key_value is not None:
                 export_settings.setValue(section_key, key_value)
         export_settings.sync()

openlp/core/ui/servicemanager.py

@@ -494,7 +494,7 @@ class ServiceManager(OpenLPMixin, RegistryMixin, QtGui.QWidget, Ui_ServiceManage
         service.append({'openlp_core': core})
         return service
 
-    def save_file(self):
+    def save_file(self, field=None):
         """
         Save the current service file.
 
@@ -1427,9 +1427,10 @@ class ServiceManager(OpenLPMixin, RegistryMixin, QtGui.QWidget, Ui_ServiceManage
         self.drop_position = -1
         self.set_modified()
 
-    def make_preview(self):
+    def make_preview(self, field=None):
         """
         Send the current item to the Preview slide controller
+        :param field:
         """
         self.application.set_busy_cursor()
         item, child = self.find_service_item()

openlp/core/ui/thememanager.py

@@ -37,7 +37,7 @@ from xml.etree.ElementTree import ElementTree, XML
 from PyQt4 import QtCore, QtGui
 
 from openlp.core.common import Registry, RegistryProperties, AppLocation, Settings, OpenLPMixin, RegistryMixin, \
-    check_directory_exists, UiStrings, translate
+    check_directory_exists, UiStrings, translate, is_win
 from openlp.core.lib import FileDialog, ImageSource, OpenLPToolbar, get_text_file_string, build_icon, \
     check_item_selected, create_thumb, validate_thumb
 from openlp.core.lib.theme import ThemeXML, BackgroundType
@@ -662,8 +662,12 @@ class ThemeManager(OpenLPMixin, RegistryMixin, QtGui.QWidget, Ui_ThemeManager, R
                 out_file.close()
         if image_from and os.path.abspath(image_from) != os.path.abspath(image_to):
             try:
-                encoding = get_filesystem_encoding()
+                # Windows is always unicode, so no need to encode filenames
-                shutil.copyfile(str(image_from).encode(encoding), str(image_to).encode(encoding))
+                if is_win():
+                    shutil.copyfile(image_from, image_to)
+                else:
+                    encoding = get_filesystem_encoding()
+                    shutil.copyfile(image_from.encode(encoding), image_to.encode(encoding))
             except IOError as xxx_todo_changeme:
                 shutil.Error = xxx_todo_changeme
                 self.log_exception('Failed to save theme image')

openlp/plugins/songs/lib/importers/worshipassistant.py

@@ -93,7 +93,7 @@ class WorshipAssistantImport(SongImport):
         details = chardet.detect(detect_content)
         detect_file.close()
         songs_file = open(self.import_source, 'r', encoding=details['encoding'])
-        songs_reader = csv.DictReader(songs_file)
+        songs_reader = csv.DictReader(songs_file, escapechar='\\')
         try:
             records = list(songs_reader)
         except csv.Error as e:
@@ -104,6 +104,8 @@ class WorshipAssistantImport(SongImport):
         num_records = len(records)
         log.info('%s records found in CSV file' % num_records)
         self.import_wizard.progress_bar.setMaximum(num_records)
+        # Create regex to strip html tags
+        re_html_strip = re.compile(r'<[^>]+>')
         for index, record in enumerate(records, 1):
             if self.stop_import_flag:
                 return
@@ -119,13 +121,12 @@ class WorshipAssistantImport(SongImport):
                 self.title = record['TITLE']
                 if record['AUTHOR'] != EMPTY_STR:
                     self.parse_author(record['AUTHOR'])
-                    print(record['AUTHOR'])
                 if record['COPYRIGHT'] != EMPTY_STR:
                     self.add_copyright(record['COPYRIGHT'])
                 if record['CCLINR'] != EMPTY_STR:
                     self.ccli_number = record['CCLINR']
                 if record['ROADMAP'] != EMPTY_STR:
-                    verse_order_list = record['ROADMAP'].split(',')
+                    verse_order_list = [x.strip() for x in record['ROADMAP'].split(',')]
                 lyrics = record['LYRICS2']
             except UnicodeDecodeError as e:
                 self.log_error(translate('SongsPlugin.WorshipAssistantImport', 'Record %d' % index),
@@ -136,8 +137,15 @@ class WorshipAssistantImport(SongImport):
                                          'File not valid WorshipAssistant CSV format.'), 'TypeError: %s' % e)
                 return
             verse = ''
+            used_verses = []
             for line in lyrics.splitlines():
                 if line.startswith('['):  # verse marker
+                    # Add previous verse
+                    if verse:
+                        # remove trailing linebreak, part of the WA syntax
+                        self.add_verse(verse[:-1], verse_id)
+                        used_verses.append(verse_id)
+                        verse = ''
                     # drop the square brackets
                     right_bracket = line.find(']')
                     content = line[1:right_bracket].lower()
@@ -152,19 +160,31 @@ class WorshipAssistantImport(SongImport):
                     verse_index = VerseType.from_loose_input(verse_tag) if verse_tag else 0
                     verse_tag = VerseType.tags[verse_index]
                     # Update verse order when the verse name has changed
-                    if content != verse_tag + verse_num:
+                    verse_id = verse_tag + verse_num
+                    # Make sure we've not choosen an id already used
+                    while verse_id in verse_order_list and content in verse_order_list:
+                        verse_num = str(int(verse_num) + 1)
+                        verse_id = verse_tag + verse_num
+                    if content != verse_id:
                         for i in range(len(verse_order_list)):
                             if verse_order_list[i].lower() == content.lower():
-                                verse_order_list[i] = verse_tag + verse_num
+                                verse_order_list[i] = verse_id
-                elif line and not line.isspace():
+                else:
-                    verse += line + '\n'
+                    # add line text to verse. Strip out html
-                elif verse:
+                    verse += re_html_strip.sub('', line) + '\n'
-                    self.add_verse(verse, verse_tag+verse_num)
-                    verse = ''
             if verse:
-                self.add_verse(verse, verse_tag+verse_num)
+                # remove trailing linebreak, part of the WA syntax
+                if verse.endswith('\n\n'):
+                    verse = verse[:-1]
+                self.add_verse(verse, verse_id)
+                used_verses.append(verse_id)
             if verse_order_list:
-                self.verse_order_list = verse_order_list
+                # Use the verse order in the import, but remove entries that doesn't have a text
+                cleaned_verse_order_list = []
+                for verse in verse_order_list:
+                    if verse in used_verses:
+                        cleaned_verse_order_list.append(verse)
+                self.verse_order_list = cleaned_verse_order_list
             if not self.finish():
                 self.log_error(translate('SongsPlugin.WorshipAssistantImport', 'Record %d') % index
                                + (': "' + self.title + '"' if self.title else ''))

tests/functional/openlp_core_common/test_settings.py

@@ -115,3 +115,15 @@ class TestSettings(TestCase, TestMixin):
 
         # THEN the new value is returned when re-read
         self.assertEqual('very short', Settings().value('test/extend'), 'The saved value should be returned')
+
+    def settings_nonexisting_test(self):
+        """
+        Test the Settings on query for non-existing value
+        """
+        # GIVEN: A new Settings setup
+        with self.assertRaises(KeyError) as cm:
+            # WHEN reading a setting that doesn't exists
+            does_not_exist_value = Settings().value('core/does not exists')
+
+        # THEN: An exception with the non-existing key should be thrown
+        self.assertEqual(str(cm.exception), "'core/does not exists'", 'We should get an exception')

tests/functional/openlp_plugins/songs/test_worshipassistantimport.py

@@ -54,3 +54,5 @@ class TestWorshipAssistantFileImport(SongImportTestHelper):
                          self.load_external_result_data(os.path.join(TEST_PATH, 'du_herr.json')))
         self.file_import(os.path.join(TEST_PATH, 'would_you_be_free.csv'),
                          self.load_external_result_data(os.path.join(TEST_PATH, 'would_you_be_free.json')))
+        self.file_import(os.path.join(TEST_PATH, 'would_you_be_free2.csv'),
+                         self.load_external_result_data(os.path.join(TEST_PATH, 'would_you_be_free.json')))

tests/resources/worshipassistantsongs/would_you_be_free.json

@@ -8,7 +8,7 @@
     "copyright": "Public Domain",
     "verses": [
         [
-            "Would you be free from your burden of sin? \nThere's power in the blood, power in the blood \nWould you o'er evil a victory win?  \nThere's wonderful power in the blood \n",
+            "Would you be free from your burden of sin? \nThere's power in the blood, power in the blood \nWould you o'er evil a victory win?  \nThere's wonderful power in the blood \n ",
             "v1"
         ],
         [

tests/resources/worshipassistantsongs/would_you_be_free2.csv

@@ -0,0 +1,31 @@
+SONGNR,TITLE,AUTHOR,COPYRIGHT,FIRSTLINE,PRIKEY,ALTKEY,TEMPO,FOCUS,THEME,SCRIPTURE,ACTIVE,SONGBOOK,TIMESIG,INTRODUCED,LASTUSED,TIMESUSED,CCLINR,USER1,USER2,USER3,USER4,USER5,ROADMAP,FILELINK1,OVERMAP,FILELINK2,LYRICS,INFO,LYRICS2,Background
+"7","Would You Be Free","Jones, Lewis E.","Public Domain","Would you be free from your burden of sin?","G","","Moderate","Only To Others","","","N","Y","","1899-12-30","1899-12-30","","","","","","","","1, C, 1","","","",".G                           C         G  
+ Would you be free from your burden of sin? 
+.        D            D7     G     
+ There's power in the blood, power in the blood 
+.                      C       G 
+ Would you o'er evil a victory win?  
+.        D         D7           G 
+ There's wonderful power in the blood 
+ 
+.G                      C              G 
+ There is power, power, wonder working power 
+.D            G 
+ In the blood of the Lamb 
+.                       C              G 
+ There is power, power, wonder working power 
+.       D                     G 
+ In the precious blood of the Lamb 
+","","[1] 
+Would you be free from your burden of sin? 
+There's power in the blood, power in the blood 
+Would you o'er evil a victory win?  
+There's wonderful power in the blood 
+ 
+[C] 
+There is power, power, wonder working power 
+In the blood of the Lamb 
+There is power, power, wonder working power 
+In the precious blood of the Lamb
+
+[x]",""